From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <libc-alpha-return-109405-listarch-libc-alpha=sources.redhat.com@sourceware.org>
Received: (qmail 125180 invoked by alias); 10 Feb 2020 19:58:57 -0000
Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id: <libc-alpha.sourceware.org>
List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org>
List-Archive: <http://sourceware.org/ml/libc-alpha/>
List-Post: <mailto:libc-alpha@sourceware.org>
List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs>
Sender: libc-alpha-owner@sourceware.org
Received: (qmail 125170 invoked by uid 89); 10 Feb 2020 19:58:56 -0000
Authentication-Results: sourceware.org; auth=none
X-Spam-SWARE-Status: No, score=-3.8 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.1 spammy=HX-Languages-Length:2244
X-HELO: albireo.enyo.de
From: Florian Weimer <fw@deneb.enyo.de>
To: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Cc: libc-alpha@sourceware.org
Subject: Re: [PATCH 1/5] Add internal <file_change_detection.h> header file
References: <cover.1579631655.git.fweimer@redhat.com>
	<691b5b8d18c29b5c31de804b8393a1b9718e1a1d.1579631655.git.fweimer@redhat.com>
	<cb0bc5a9-a0bd-7583-00b5-1151c31a34fe@linaro.org>
Date: Mon, 10 Feb 2020 19:58:00 -0000
In-Reply-To: <cb0bc5a9-a0bd-7583-00b5-1151c31a34fe@linaro.org> (Adhemerval
	Zanella's message of "Mon, 10 Feb 2020 16:47:04 -0300")
Message-ID: <87v9oez03z.fsf@mid.deneb.enyo.de>
MIME-Version: 1.0
Content-Type: text/plain
X-SW-Source: 2020-02/txt/msg00311.txt.bz2

* Adhemerval Zanella:

> On 21/01/2020 15:41, Florian Weimer wrote:
>> The code started out with bits form resolv/resolv_conf.c, but it
>> was enhanced to deal with directories and FIFOs in a more predictable
>> manner.  A test case is included as well.
>> 
>> This will be used to implement the /etc/resolv.conf change detection.
>> 
>> This currently lives in a header file only.  Once there are multiple
>> users, the implementations should be moved into C files.
>
> LGTM, with some nits below.
>
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>

Thanks.
>> +static void
>> +all_same (struct file_change_detection *array, size_t length)
>> +{
>> +  for (size_t i = 0; i < length; ++i)
>> +    for (size_t j = 0; j < length; ++j)
>> +      {
>> +        if (test_verbose > 0)
>> +          printf ("info: comparing %zu and %zu\n", i, j);
>> +        TEST_VERIFY (file_is_unchanged (array + i, array + j));
>> +      }
>> +}
>> +
>
> Wouldn't it the following be slight better?
>
>   for (size_t i = 0; i < length - 1; ++i)
>     for (size_t j = i + 1 < length, j++)
>        
> Or do you really to check all possible permutations?

Iterating over all pairs checks that the operation is commutative.

>> +  /* Wait for a file change.  Depending on file system time stamp
>> +     resolution, this subtest blocks for a while.  */
>> +  for (int use_stdio = 0; use_stdio < 2; ++use_stdio)
>> +    {
>> +      struct file_change_detection initial;
>> +      TEST_VERIFY (file_change_detection_for_path (&initial, path_file1));
>> +      while (true)
>> +        {
>> +          support_write_file_string (path_file1, "line\n");
>> +          struct file_change_detection current;
>> +          if (use_stdio)
>> +            TEST_VERIFY (file_change_detection_for_fp (&current, fp_file1));
>> +          else
>> +            TEST_VERIFY (file_change_detection_for_path (&current, path_file1));
>> +          if (!file_is_unchanged (&initial, &current))
>> +            break;
>> +          /* Wait for a bit to reduce system load.  */
>> +          usleep (100 * 1000);
>> +        }
>> +    }
>
> Ok, although the usleep seems excessive large (the testing will most likely
> timeout prior usleep return).

Hmm, I thought that this would wait 100 milliseconds?  So the loop
should exit in a second or two with low-resolution timestamps in the
file system.