[PATCH v2 1/2] [libXau] XauGetFileName: added a thread-safe variant of XauFileName

[Erkki Seppala]

On 31.03.2011 16:13, Mark Kettenis wrote:
> Sorry, but I think that is a bad API.  It will lead to confusion
> who allocated the buffer.

I would hope the documentation makes this clear. Unfortunately this is 
that kind of API where one wants to look at the documentation :).

> Also somebody noted that that the only external program thet uses
> XauFileName(3) is xauth(1).  So I think there is no good reason to
> export the new thread-safe interface and only use it internally.  That
> makes my criticism about the chosen API less important, so feel free
> to ignore those objections if you decide not to export this new interface.

The design was based on the idea that it is good to have an API where 
the caller provides the storage, but given that in the typical use case 
the caller doesn't have a storage of proper size, it is convenient to 
have the function allocate the memory it needs by itself. Perhaps the 
problem is trying to fit both use cases into the same function?

Now, without the dynamic allocation part the caller could first call 
this function with buffer_size = 0 at first and that way acquire the 
required buffer size and then call it again with the real buffer to 
acquire the actual file name, but this complicates the most common use 
case. (For example the fixes to use this API were for XauGet*AuthByAddr 
were quite simple.) What really would happen is that the developer would 
allocate a "large enough" buffer (say, PATH_MAX) and run with that. But 
PATH_MAX isn't a good solution, as it isn't available everywhere.

An another, perhaps better, alternative would be to have a yet another 
function, such as XauAllocAndGetFileName, which would then be used at 
the two call sites inside libXau (and likely everywhere else), leaving 
XauFileName to be the only direct user of XauGetFileName. But even 
XauFileName could just get rid of its static buffer and maintain a 
dynamically allocated static pointer that gets dynamically allocated on 
every invocation -- leaving no users of the fixed buffer API, which 
could then be removed.

> See my other diff about why I don't think supporting the NetBSD
> getenv_r(3) is worth it.

I wonder if any system then actually has non-thread-safe getent 
(disregarding the case of concurrent putenv)..