[PATCH] support: Add support for delayed test failure reporting

[Florian Weimer]

On 12/28/2016 02:04 PM, Adhemerval Zanella wrote:
>
>> +/* This structure keeps track of test failures.  The counter is
>> +   incremented on each failure.  The failed member is set to true if a
>> +   failure is detected, so that even if the counter wraps around to
>> +   zero, the failure of a test can be detected.
>> +
>> +   The init constructor function below puts *state on a shared
>> +   annonymous mapping, so that failure reports from subprocesses
>> +   propagate to the parent process.  */
>> +struct test_failures
>> +{
>> +  unsigned counter;
>> +  unsigned failed;
>
> Shouldn't these use 'unsigned int' based on code style?

You are correct.

>> +};
>> +static struct test_failures *state;
>> +
>> +static __attribute__ ((constructor)) void
>> +init (void)
>> +{
>> +  void *ptr = mmap (NULL, sizeof (*state), PROT_READ | PROT_WRITE,
>> +                    MAP_ANONYMOUS | MAP_SHARED, -1, 0);
>> +  if (ptr == MAP_FAILED)
>> +    {
>> +      printf ("error: could not map %zu bytes: %m\n", sizeof (*state));
>> +      exit (1);
>> +    }
>
> Why not use FAIL_EXIT1 (also for other failures cases as well)?

We might want to add implicit delayed error reporting to FAIL_EXIT1 
eventually, so that it works from subprocess as well.  In that case, 
this would introduce a circular dependency.

>> +  /* Check that the status is passed through unchanged.  */
>> +  check_failure_reporting (1, 0, EXIT_UNSUPPORTED);
>
> I am not very found of plain number to indicate the arguments (it tends to make
> code harder to read), so I would suggest for a future clean up to add either some
> enum or defined to indicate the phase (like PHASE{1,...}) and same for the zero
> argument.

The phase number is just there so that it easier to find the exact place 
where a test failure occurred.  I could perhaps replace it with 
__LINE__, or maybe with a global variable.  Adding an enum isn't really 
helpful because the values have no intrinsic meaning.  I agree that the 
collection of int argument is a bit unfortunate.

Thanks,
Florian