[PATCH] parse-datetime: use labs for long int

[Ruediger Meier]

From: Ruediger Meier <***@ga-group.nl>

clang warns about this:

lib/parse-date.y:909:16: warning: absolute value function 'abs' given an
argument of type 'long' but has parameter of type 'int' which may cause
truncation of value [-Wabsolute-value]

Signed-off-by: Ruediger Meier <***@ga-group.nl>
---
lib/parse-datetime.y | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/parse-datetime.y b/lib/parse-datetime.y
index 51ba4cc..b3a872b 100644
--- a/lib/parse-datetime.y
+++ b/lib/parse-datetime.y
@@ -1154,7 +1154,7 @@ time_zone_hhmm (parser_control *pc, textint s, long int mm)
/* If the absolute number of minutes is larger than 24 hours,
arrange to reject it by incrementing pc->zones_seen. Thus,
we allow only values in the range UTC-24:00 to UTC+24:00. */
- if (24 * 60 < abs (n_minutes))
+ if (24 * 60 < labs (n_minutes))
pc->zones_seen++;

return n_minutes;

[Paul Eggert]

Thanks for the heads-up. That code has long been on my list of things to clean
up, and your message prompted me to go about it. I installed the attached, which
fixes the runtime problem corresponding to the diagnostic you mentioned, along
with a lot of other problems. Please give it a try. I hope that your clang is
smart enough not to complain about expressions like abs (x % 60) merely because
x's data type is wider than int.

[Rüdiger Meier]

Post by Paul Eggert
Thanks for the heads-up. That code has long been on my list of things to
clean up, and your message prompted me to go about it. I installed the
attached, which fixes the runtime problem corresponding to the
diagnostic you mentioned, along with a lot of other problems. Please
give it a try.

Thanks a lot, I've reviewed your patch a bit and it looks good. But I
can't test my particular case before next week or so.

I hope that your clang is smart enough not to complain

Post by Paul Eggert
about expressions like abs (x % 60) merely because x's data type is
wider than int.

The compiler would still warn about "abs( ((long)x) % 60 )" but actually
I can't find any place anymore in the new code where x is a long. So all
should be fine now.

cu,
Rudi

[Pádraig Brady]

Wow impressive improvements!

(date): Fix printf of size_t to use %z.

In coreutils we explicitly disallow the C99 %j and %z specifiers
due to portability issues on solaris 8 for example:
http://git.sv.gnu.org/cgit/coreutils.git/commit/?id=v8.23-75-g7d1fe88

In general gnulib is still targeting c89 right?
BTW, when should we update that requirement?

Anyway depending on the printf module would be overkill
to cater for this, so using PRIuMAX may be best?

cheers,
Pádraig

[Paul Eggert]

Post by Pádraig Brady
using PRIuMAX may be best?

That would be too easy :-). I installed the attached.

[Paul Eggert]

Post by Pádraig Brady
In general gnulib is still targeting c89 right?
BTW, when should we update that requirement?

Now is a good time. As far as I know, no Gnulib-using application still requires
porting to C89-only platforms. Although we still may have some issues with old C
libraries that support C89 but not C99 behavior, we no longer need to worry
about porting to C89-only compilers. In other words, all platforms that we care
about now have a compiler available that supports significant C99 features (even
if it does not support full C99).

With that in mind, I propose (and have installed) the attached patch to try to
document the current situation more accurately. Comments and further patches are
welcome.

[Tim Rice]

Post by Paul Eggert

Post by Pádraig Brady
In general gnulib is still targeting c89 right?
BTW, when should we update that requirement?

Now is a good time. As far as I know, no Gnulib-using application still
requires porting to C89-only platforms. Although we still may have some issues
with old C libraries that support C89 but not C99 behavior, we no longer need
to worry about porting to C89-only compilers. In other words, all platforms
that we care about now have a compiler available that supports significant C99
features (even if it does not support full C99).

Sad to hear gnulib doesn't care about C89. UnixWare (a currently shipping
product) has a C89 compiler.
I was hoping to find time to upstream my patch to get UnixWare farther along
with gnulib but I guess it's wasted time now.
Such is life with long lived products.

Post by Paul Eggert
With that in mind, I propose (and have installed) the attached patch to try to
document the current situation more accurately. Comments and further patches
are welcome.

[Paul Eggert]

Post by Tim Rice
Sad to hear gnulib doesn't care about C89. UnixWare (a currently shipping
product) has a C89 compiler.

That should be OK as UnixWare's supplier also supports GCC, and recommends GCC
for compiling Gnu-style applications.

http://osr507doc.xinuos.com/en/DevSysoview/Ccompil.html#Ccompil_GNUdevtools

Plus, the native UnixWare compiler supports many C99 features, so it may work
with even the parts of Gnulib that use some C99 now.

http://osr507doc.xinuos.com/en/DevSysoview/Ccompil.html#Ccompil_OSR5devsys

[Tim Rice]

Hi Paul,

Post by Paul Eggert

Post by Tim Rice
Sad to hear gnulib doesn't care about C89. UnixWare (a currently shipping
product) has a C89 compiler.

That should be OK as UnixWare's supplier also supports GCC, and recommends GCC
for compiling Gnu-style applications.
http://osr507doc.xinuos.com/en/DevSysoview/Ccompil.html#Ccompil_GNUdevtools
Plus, the native UnixWare compiler supports many C99 features, so it may work
with even the parts of Gnulib that use some C99 now.
http://osr507doc.xinuos.com/en/DevSysoview/Ccompil.html#Ccompil_OSR5devsys

You are quoting the OpenServer 5 documentation (different compiler) but
digging deeper into the UnixWare web docs I see it supports most of C99 too.

Sorry for the noise.

[Bruno Haible]

UnixWare (a currently shipping product) has a C89 compiler.

This product appears to be of zero relevance in the market: I haven't seen any
reference to it in mailing lists or bug reports since ca. 1997.

Currently shipping? The version number is at 7.1.4 since 2004 [1], and the
"new features" of the current release are, in particular, "USB 2.0 support" [2].

Enough said.

Bruno

[1] https://en.wikipedia.org/wiki/UnixWare#Version_history
[2] http://sco.com/products/unixware714/index.html

[Pádraig Brady]

Post by Paul Eggert
Thanks for the heads-up. That code has long been on my list of things to clean

FYI I'm currently running date(1) with this change
+ ASAN enabled, through the latest AFL fuzzer,
seeded with the coreutils test corpus.