Re: test-fdutimensat racy?

[Pádraig Brady]

On 06/04/2013 09:20 PM, Bernhard Voelker wrote:
> On 05/22/2013 12:18 AM, Bernhard Voelker wrote:
>> A new patch follows below.  I also added a note about the maximum
>> delay as Padraig suggested.
> 
> I changed nap()'s limit for nanosleep from 2000000000 to the correct
> number (2^31-1), and added a Changelog entry, as well as rebasing against
> latest Git.
> 
> Have a nice day,
> Berny
> 
>>From 6572ae5fcba1b39b495888fd512b20f54c0d1545 Mon Sep 17 00:00:00 2001
> From: Bernhard Voelker <address@hidden>
> Date: Tue, 4 Jun 2013 22:18:31 +0200
> Subject: [PATCH] tests/nap.h: avoid race

That's a bit terse. I'd instead say:
tests/nap.h: use an adaptive delay to avoid ctime update issues

It might be worth referencing some of the threads describing
the possible kernel issues, like:
https://lists.gnu.org/archive/html/bug-gnulib/2011-11/msg00226.html

> +static void
> +clear_temp_file (void)
>  {

> +  ASSERT (0 <= nap_fd);
> +  ASSERT (close (nap_fd) != -1);
> +  ASSERT (unlink (TEMPFILE) != -1);
>  }

It might be better to:

if (0 <= nap_fd)
  {
    ASSERT (close (nap_fd) != -1);
    ASSERT (unlink (TEMPFILE) != -1);
  }

which would allow you to setup the atexit handler
before the file is created, thus closing any
small race between creating the file and
enabling the cleanup function.

> +  for ( ; delay <= 2147483647; delay = delay * 2)
> +    if (nap_works (nap_fd, delay, old_st))
> +      return;

nice one.

thanks,
Pádraig.