[PATCH] libc/time/gmtime_r.c, libc/time/lcltime_r.c,, libc/time/local.h, libc/time/mktm_r.c: move localtime related functionality, from _mktm_r() to new _mklocaltm_r() to break dependency of gmtime() on, timezones

[Corinna Vinschen]

Hi Freddie,

On Sep 11 22:18, Freddie Chopin wrote:
> W dniu 2014-09-11 21:51, Jeff Johnston pisze:
> >Hi Freddie,
> >
> >Corinna who was looking at this previously is on vacation so I am reviewing it.
> 
> Good to know, thanks for your review!
> 
> >The only comments I have are:
> >
> >1. month_lengths needs to be renamed to be __month_lengths because it is now externalized.
> >2. you might as well be consistent with other files in LIB_SOURCES list of Makefile.am with regards to tabs vs spaces.
> 
> Both issues fixed. Updated patch and updated changelog attached.
> 
> Regards,
> FCh

> 2014-09-11  Freddie Chopin  <freddie_chopin@op.pl>
> 
> 	* libc\time\month_lengths.c: New file with __month_lengths array
> 	(previously mon_lengths array in mktm_r.c)
> 	* libc\time\tzcalc_limits.c: New file with __tzcalc_limits() from
> 	mktm_r.c
> 	* libc\time\lcltime_r.c (localtime_r): Simplify by changing call to
> 	_mktm_r() with call to gmtime_r() and code moved from _mktm_r() which
> 	was used to do time zone adjustments
> 	* libc\time\gmtime_r.c (gmtime_r): Simplify by moving all relevant
> 	code from _mktm_r(), breaking all dependencies on time zone related
> 	functions
> 	* libc\time\mktm_r.c: Delete file
> 	* libc\time\local.h: Update accordingly - remove declaration of
> 	_mktm_r(), add declaration of __month_lengths[]
> 	* libc\time\Makefile.am: Modify accordingly.
> 	* libc\time\Makefile.in: Regenerate.

With this patch applied, a Coverity scan shows this new problem:

  *** CID 74279:  Logically dead code  (DEADCODE)
  /newlib/libc/time/gmtime_r.c: 52 in gmtime_r()
  46         {
  47           rem += SECSPERDAY;
  48           --days;
  49         }
  50       while (rem >= SECSPERDAY)
  51         {
  >>>     CID 74279:  Logically dead code  (DEADCODE)
  >>>     Execution cannot reach this statement: "rem -= 86400L;".
  52           rem -= SECSPERDAY;
  53           ++days;
  54         }
  55
  56       /* compute hour, min, and sec */
  57       res->tm_hour = (int) (rem / SECSPERHOUR);

AFAICS, Coverity is right.  The code starts with

  rem = ((long)lcltime) % SECSPERDAY;

thus lcltime is always < SECSPERDAY.  Is that just an oversight in
removing some unused code, or does that point to some other problem,
perhaps?


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/newlib/attachments/20141014/dbea3a04/attachment.sig>