[PATCH 2/2] resolv: Avoid duplicate query if search list contains '.' (bug 33804)

[Carlos O'Donell]

On 3/2/26 12:57 PM, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>>> I thought it was required because _THROW was used in NSS function
>>> declarations (which implies attribute leaf).  But the tested functions
>>> do not actually have this, so I can drop the volatile.  (But similar
>>> constructs are used in other resolver tests, maybe also unnecessarily.)
>>
>> You are correct that __THROW implies __attribute__ ((__leaf__)) within the
>> NSS functions, which tells the compiler they do not call back into the TUs
>> definitions. They do call back though via response()? So you marked them
>> volatile for that purpose? I had not considered this aspect of the
>> implementation when I reviewed this, so I think you probably have to keep
>> the volatile.
>>
>> Yes, regarding __THROW, gethostbyname, gethostbyname2, and getaddrinfo are
>> all cancellation points and so are not marked __THROW, so they *can* call
>> back into the caller's TU and modify data... that means volatile is not
>> strictly required.
>>
>> Do we still consider these two distinct issues?
>>
>>   * Remove use of volatile because none of the called functions are __THROW?
>>   * Addition of atomics to create synchronizes with behaviour to observe results?
> 
> Atomics are not needed because the DNS packet exchange provides
> synchronization.  The DNS interaction completes before the getaddrinfo
> etc. calls return to the main program.
Within the server thread there is a call to response_callback, and during
the execution of that function the global variable is altered along with
the response buffer.

I concur that the writev() from the server thread is sufficient to
synchronize the contents of the buffer, but I don't see a strong guarantee
that the global you just added has such a guarantee?

I think atomics are not needed for the contents of the results because
both writev() and sentdo() ensure the contents are visible to the receiving
thread, but I see no such argument for the memory increment.

My position continues to be that we're just getting lucky and that eventually
we'll see failures in this test from first principles, but it depends on
the hardware.

You've addressed my concerns here and we've talked it out, and the the change
is an incremental improvement. I'm willing to see this committed.

Do we need a v2 with volatile removed because none of these functions are
__THROW?

-- 
Cheers,
Carlos.