Re: [PATCH v2 2/6] support: Add timespec-related helper functions

[Mike Crowe <mac at mcrowe dot com>]

On Tuesday 23 April 2019 at 14:12:56 -0300, Adhemerval Zanella wrote:
> > diff --git a/support/timespec-add.c b/support/timespec-add.c
> > new file mode 100644
> > index 0000000..523eac8
> > --- /dev/null
> > +++ b/support/timespec-add.c
> > @@ -0,0 +1,75 @@
> > +/* Add two struct timespec values.
> > +
> > +   Copyright (C) 2011-2019 Free Software Foundation, Inc.
> > +
> > +   This file is part of the GNU C Library, although it originally came
> > +   from gnulib.
> 
> Based from previous gnulib additions, I think we can use default Glibc
> header and maybe added a comment below that it came from gnulib
> originally.

posix/getopt.c and other files contain:

 This file is part of the GNU C Library and is also part of gnulib.
 Patches to this file should be submitted to both projects.

followed by the standard GNU C copyright notice. Would that be acceptable?

> > +/* Written by Paul Eggert.  */
> 
> We don't add authorship anymore on new files. I think a better approach
> is to comment it came from gnulib to keep track for future syncs.

OK.

> > diff --git a/support/timespec.c b/support/timespec.c
> > new file mode 100644
> > index 0000000..c34fe73
> > --- /dev/null
> > +++ b/support/timespec.c
[snip]
> > +void
> > +test_timespec_before_impl (const char *file, int line,
> > +			   const struct timespec left,
> > +			   const struct timespec right)
> > +{
> > +  if (left.tv_sec > right.tv_sec
> > +      || (left.tv_sec == right.tv_sec
> > +	  && left.tv_nsec > right.tv_nsec)) {
> > +    const int saved_errno = errno;
> > +    support_record_failure ();
> > +    const struct timespec diff = timespec_sub (left, right);
> > +    printf ("%s:%d: %ld.%09lds not before %ld.%09lds "
> > +	    "(difference %ld.%09lds)n",
> > +	    file, line,
> > +	    left.tv_sec, left.tv_nsec,
> > +	    right.tv_sec, right.tv_nsec,
> > +	    diff.tv_sec, diff.tv_nsec);
> > +    errno = saved_errno;
> > +  }
> > +}
> 
> I think there is no need to save and restore errno, unless you are
> actively testing errno after an expected failure (which is not the
> case for the patchset and it not usual for libsupport and/or glibc
> tests as well).

I can remove that. I only did so because support_test_compare_failure did
so.

Mike.