lib/util:tests/time.c - "test_timestring" - fix it on Solaris

[Jelmer Vernooij]

On Wed, 2010-12-22 at 11:40 +0100, Matthias Dieter Wallnöfer wrote:
> As far as I've seen it's used by (debug) messages and comments. And the 
> output is never reworked further. So are you okay if I merge?
+1, thanks for fixing this.

Cheers,

Jelmer
> Jelmer Vernooij wrote:
> > Hi Matthias,
> >
> > On Wed, 2010-12-22 at 10:13 +0100, Matthias Dieter Wallnöfer wrote:
> >    
> >> another patch proposal. This simply specifies more precisely the output
> >> of the date and it works on both Solaris and Linux (and I bet on other
> >> Unix systems as well). In my eyes this should be definitely okay for
> >> passing "samba4.local.time".
> >>      
> > That seems reasonable to me. Have you checked how strict the callers of
> > timestring() rely on its format?
> >
> > Cheers,
> >
> > Jelmer
> >
> >    
> >>> diff --git a/lib/util/time.c b/lib/util/time.c
> >>> index a1f2b08..770ebc4 100644
> >>> --- a/lib/util/time.c
> >>> +++ b/lib/util/time.c
> >>> @@ -387,7 +387,7 @@ _PUBLIC_ char *timestring(TALLOC_CTX *mem_ctx,
> >>> time_t t)
> >>>          /* Some versions of gcc complain about using some special format
> >>>           * specifiers. This is a bug in gcc, not a bug in this code. See a
> >>>           * recent strftime() manual page for details. */
> >>> -       strftime(tempTime,sizeof(tempTime)-1,"%c %Z",tm);
> >>> +       strftime(tempTime,sizeof(tempTime)-1,"%a %b %e %X %Y %Z",tm);
> >>>          TimeBuf = talloc_strdup(mem_ctx, tempTime);
> >>>   #else
> >>>          TimeBuf = talloc_strdup(mem_ctx, asctime(tm));
> >>>        
> >> Cheers,
> >> Matthias
> >>
> >> Matthias Dieter Wallnöfer wrote:
> >>      
> >>> Okay, I thought such a little difference (the "0" or not) wouldn't
> >>> matter - but I will revert.
> >>>
> >>> Matthias
> >>>
> >>> Jelmer Vernooij wrote:
> >>>        
> >>>> On Mon, 2010-12-06 at 11:38 +0300, Matthieu Patou wrote:
> >>>>          
> >>>>> I guess you have to introduce a test in  lib/util/wscript_configure to
> >>>>> define
> >>>>> HAVE_CORRECT_STRFTIME by testing the output of strftime() and if the
> >>>>> output is not ok then we should use asctime (at least on sun8 it
> >>>>> seems ok).
> >>>>>            
> >>>> I think we should rather try to keep all platform-specific workarounds
> >>>> in lib/replace. So if the strftime() doesn't do what we expect it to I
> >>>> think we should add a strftime() replacement to lib/replace.
> >>>>
> >>>> Cheers,
> >>>>
> >>>> Jelmer
> >>>>
> >>>>          
> >>>>> Have a look at changeset 4ea7d4694a8353fc55ecd12cb09b9c91ffde7b3f
> >>>>> On 06/12/2010 05:59, Andrew Bartlett wrote:
> >>>>>
> >>>>> Matthieu.
> >>>>>            
> >>>>>> On Sun, 2010-12-05 at 23:10 +0100, Matthias Dieter Wallnöfer wrote:
> >>>>>>              
> >>>>>>> The branch, master has been updated
> >>>>>>>           via  654e010 lib/util:tests/time.c - "test_timestring" -
> >>>>>>> fix it on Solaris
> >>>>>>>          from  55dba7b s4:cluster/cluster.h - fix another gcc 3.4
> >>>>>>> "struct" warning
> >>>>>>>
> >>>>>>> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> >>>>>>>
> >>>>>>>
> >>>>>>> - Log
> >>>>>>> -----------------------------------------------------------------
> >>>>>>> commit 654e0102ddb0acaaf45fb55c15818722235fcc9f
> >>>>>>> Author: Matthias Dieter Wallnöfer<mdw at samba.org>
> >>>>>>> Date:   Sun Dec 5 22:20:06 2010 +0100
> >>>>>>>
> >>>>>>>        lib/util:tests/time.c - "test_timestring" - fix it on Solaris
> >>>>>>>
> >>>>>>>        Solaris returns "Thu Jan 01" and not "Thu Jan  1" -
> >>>>>>> therefore proof for
> >>>>>>>        both.
> >>>>>>>                
> >>>>>> I'm not sure that this patch is correct.
> >>>>>>
> >>>>>> Given we had a test for a particular value, and we carefully build the
> >>>>>> value with strftime() if available, why does this patch accept
> >>>>>> different
> >>>>>> values, rather than fix the the timestring() function?
> >>>>>>
> >>>>>> In general, I'm suspicious of patches that fix tests by changing the
> >>>>>> test result rather than fixing the original function.
> >>>>>>
> >>>>>> Perhaps none of the callers care, as long as it's clear to human
> >>>>>> readers
> >>>>>> of the string, but there are rather a lot of callers to this function.
> >>>>>>
> >>>>>> Andrew Bartlett
> >>>>>>
> >>>>>>              
> >>>>          
> >>>
> >>>        
> >>      
> >    
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20101222/4aee4067/attachment.pgp>