[PATCH v2 1/2] [libXau] XauGetFileName: added a thread-safe variant of XauFileName
erkki.seppala at vincit.fi
Thu Mar 31 07:05:43 PDT 2011
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)..
More information about the xorg-devel