[tz] suggestions for potential code improvements?

Paul Eggert eggert at cs.ucla.edu
Fri Jul 24 16:10:44 UTC 2015


Kees Dekker wrote:
> Hi Paul,
>
> I tried to compile the code (Visual Studio 2013 as first try, the other platforms will follow later), but I found some (minor) things:
>
> 1. The implementation of difftime() is specified with const arguments, while none of the referred places use these const qualifiers (e.g. private.h:368).
...

> Const mismatches:
> 3. There are (static) prototype mismatches between strftime.c:99 and the implementation of _fmt (arguments 2 and 4 have additional const in definition)
> 4.  idem for strftime.c:98 and the implementation of _conv() at line 572 (arguments 1, 2, 3 and 4 have additional const qualifiers)
> 5.  idem for strftime.c:97 and the implementation of _add() at line 580 (arguments 3 and 6 are different from declaration)
> I don't really prefer one or other, but usually a compiler may optimize const qualified code somewhat better.

That shouldn't matter.  If the argument to a function is declared 'const', the 
'const' attribute matters only in the body of the function; it is irrelevant to 
callers.  This has been true ever since C was standardized in C89.  No decent 
compiler should generate different code because of the extra 'const's here.

That being said, I'd rather omit 'const' when used with locals as it's not 
useful enough to justify the code bloat.  Since it's just a style thing and 
VS2013 is complaining we might as well omit it, so I installed the attached 
proposed patch in the experimental version on github 
<https://github.com/eggert/tz>; please give it a try.

> 2. The code in difftime.c:29, 37 and 50

Perhaps you're using some old version of difftime.c?  Ours has only 48 lines.  I 
suggest trying the latest version, preferably the experimental version on github.

> (a subtraction of two doubles) results in a: conversion from 'const time_t' to 'double', possible loss of data warning.

'difftime' returns 'double', so (on some platforms, anyway) data will be lost. 
We can't change the return type of 'difftime' -- it's part of the C standard -- 
so people will just have to live with the data loss.

> May be Visual Studio is somewhat pedantic in this case, but a cast will solve the issue.

Is the idea to change "return time1 - time0;" with "return (double) (time1 - 
time0);"?  If so, I'd rather not.  Casts are powerful in C and can lead to 
programming errors, so if there are two valid ways to write code, one with casts 
and one without, we should use the latter even if VS2013 complains about it.

> If you like, I will also check the other files, but that will take additional time.

It shouldn't take long, no?  Just compile the files and email us the compiler's 
messages.  Should take less than a minute.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Pacify-Visual-Studio-2013.patch
Type: text/x-diff
Size: 5362 bytes
Desc: not available
URL: <http://mm.icann.org/pipermail/tz/attachments/20150724/0b0999e7/0001-Pacify-Visual-Studio-2013.patch>


More information about the tz mailing list