Re: [PATCH i-g-t 1/3] lib/core: Avoid unused result in backtrace printing

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 03/11/2017 13:02, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2017-11-03 12:56:14)
>>
>> On 03/11/2017 12:10, Chris Wilson wrote:
>>> Quoting Tvrtko Ursulin (2017-11-03 12:04:15)
>>>>
>>>> On 03/11/2017 11:52, Chris Wilson wrote:
>>>>> Quoting Tvrtko Ursulin (2017-11-03 11:47:54)
>>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>>
>>>>>> Add helpers for direct write to stderr to consolidate the code
>>>>>> and avoid the unused result warning in build.
>>>>>>
>>>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>> ---
>>>>>>     lib/igt_core.c | 23 ++++++++++++++++-------
>>>>>>     1 file changed, 16 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/lib/igt_core.c b/lib/igt_core.c
>>>>>> index 538a4472e209..351859eaa04c 100644
>>>>>> --- a/lib/igt_core.c
>>>>>> +++ b/lib/igt_core.c
>>>>>> @@ -1239,7 +1239,7 @@ static const char hex[] = "0123456789abcdef";
>>>>>>     static void
>>>>>>     xputch(int c)
>>>>>>     {
>>>>>> -       write(STDERR_FILENO, (const void *) &c, 1);
>>>>>> +       igt_assert_eq(write(STDERR_FILENO, (const void *) &c, 1), 1);
>>>>>
>>>>> Infinite recursion, you can't use an assert from inside the assert
>>>>> handler.
>>>>
>>>> I thought it is the signal handler. Tested sending some signals and it
>>>> survived. If there is a flaw there could instead nerf them with void
>>>> cast, don't know.
>>>
>>> I do recall these are meant to be fail safe, so using something like
>>> assert which may end up here again is no-no. igt_ignore_warn()?
>>
>> But then on the other hand igt_fail seems to have handling for exit
>> handlers using igt_asserts. Which suggests what I've done might be fine
>> after all.
> 
> Otoh, where does the assert go if we fail to write to stderr? I do not
> like the means by which we are meant to be writing the death throes to
> cause further output. These routines are the lowest level and meant to
> be impervious, callable from any context, calling back up into igt is a
> layering violation.

I can send a version which avoids that, but also note fatal_sig_handler 
already uses igt_assert. I'll end up changing more than I bargained for 
when I started. :)

Regards,

Tvrtko

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx