Re: [PATCH 1/2] Merge mktime, timegm from upstream Gnulib

[Albert ARIBAUD <albert dot aribaud at 3adev dot fr>]

Hi Paul,

On Mon, 8 Oct 2018 16:23:35 -0700, Paul Eggert <eggert@cs.ucla.edu>
wrote :

> On 10/4/18 4:41 AM, Albert ARIBAUD wrote:
> > Any clue as to why this happens?  
> 
> I'm not quite sure what you meant by "above release/2.28.master" and "on 
> release/2.28.master just below the committed patch". However, one 
> possible explanation for an infloop if the patch is improperly installed 
> so that time/mktime.c is compiled for glibc without _LIBC being defined; 
> in that case mktime's body could be just '__tzset (); return mktime 
> (tp);' which would infloop. You can investigate this by disassembling 
> mktime.

What I meant (apart from the typo in the branch name, sorry) is that I
had checked out the glibc branch 'release/2.28/master' into a local
branch of mine, and then applied your patch, extracted from your post
to the mailing list on September 9th at 18:06.

I have just compared this patch to the one obtained by running a 'git
diff HEAD^' at commit 8e6fd2bd, which contains your change as applied
to the glibc master branch, and I see one difference: the mailing
list diff has a line in the time/mktime.c part which reads

	+# if defined __LIBC || NEED_MKTIME_WORKING

whereas the git diff HEAD^ from commit 8e6fd2bd has the line

	# if defined _LIBC || NEED_MKTIME_WORKING

Clearly there was a change in the number of underscores before "LIBC",
two in the mailing list version, one in the git version; this is the
only difference between the two diffs.

The "__LIBC" in the patch appears to be a typo, and does in fact make
mktime() call itself recursively when building for glibc, consistent
with the freeze I encountered.

I've just triple-checked the mailing list to make sure I did not
miss any reply to your patch or any announcement of it being hot-fixed
while applied, or any post at all regarding that typo. There was none,
so I had no way of knowing that the patch was in fact incorrect.

Now that I have identified the typo, and fixed it in my own branch, I
have re-run make check and got exactly the same results as the 2.28
master branch gets.

Cordialement,
Albert ARIBAUD
3ADEV