Re: Leak in XKeysymToString

[Alan Coopersmith]

On 8/21/22 18:20, Po Lu wrote:

Alan Coopersmith  writes:


Okay, but we also document that Xlib is thread safe if XInitThreads() has
been called, so both the patch suggested here to keep a static pointer to
a malloc'ed buffer and my suggestion of a global static buffer fail since
calls in different threads would have a race condition over whose answer
got returned.   We'd at least need a thread-specific buffer, which it
doesn't look like we've done in Xlib so far.


In modern C that is as simple as using _Thread_local, I think.


I don't know if all our supported platforms support C11 yet - most will,
but even getting to full C99 support took years with holdouts from certain
NetBSD & OpenBSD platforms, and Microsoft's compilers.

(This is a code base that predates C89 by 5 years and was originally
 written to K&R C, so C99 is pretty modern for us.)

--
-Alan Coopersmith- alan.coopersm...@oracle.com
 Oracle Solaris Engineering - https://blogs.oracle.com/solaris

Re: Leak in XKeysymToString

[Po Lu]

Alan Coopersmith  writes:

> Thanks - while gitlab is our preferred method, when that's not possible,
> we prefer using the xorg-devel mailing list (cc'ed) instead of trying to
> guess which individual developer to contact.

Thanks for explaining.  I thought xorg-devel was shut down and replaced
by Discourse, but I'm probably confusing it with GNOME lists.

> This bug has been previously reported, but no one has developed a good
> fix yet - I don't know if many XKeysymToString callers keep references to
> the returned pointers and would be broken if those pointers suddenly had a
> different string or were invalid due to a realloc() call.

FWIW, libxkbfile (which triggers this bug) does not.  I haven't seen
code do that myself either.

> If they do, then perhaps a safer fix would be to keep a list of the
> returned strings and reuse the pointer when the same keysym is seen again
> instead of continuing to allocate new memory each time.

Probably in an assoc table or similar structure, right?  Using a plain
list seems horribly inefficient once XKeysymToString is called for
enough keysyms.

> The safest fix would be to just have makekeys generate these values
> when building the keysym tables, but that would increase the memory
> usage for those tables.

That could work too, yes.

> If not, and we decided overwriting the string each time was acceptable,
> I'd just go with "static char s[10];" instead of malloc/realloc at all.

Sure.