I'm aware that GTK2 is a bit of a dark and dusty corner in Pike, with few devs understanding everything that goes on in there. Nonetheless, I'm hoping someone can eyeball my code, at least from a general Pike dev point of view. Are there pitfalls that I haven't noticed? Things that I've done inefficiently?
As mentioned in another thread, I've used some C99 here. Is this now permitted? If not, I can rejig that.
If nobody has any comments (at least this time I know to keep an eye on LysKOM!), I'll merge it into 8.1, but I'd rather have at least one additional pair of eyeballs run over the code before that happens, just in case.
ChrisA
Responding to Martin Nilsson's post on LysKOM (if I respond directly there, I'll have no record at my end):
From a very quick look: _decode_targets in gtkwidget.pre doesn't check that the sub-array size is 3 before indexing it. And checking that ITEM(cur)[0].u.string->size_shift is 0 is easy to add. Are there any other restrictions on the string (e.g. is \0 allowed)? The new string range functions are handy for that.
Oops, good catch on the sub-array size check. And yeah, added the assertion that size_shift be zero; although I'm not sure whether I should be UTF-8 encoding (which may apply even if size_shift is nonzero) or handling non-ASCII strings in some other way.
My guess is that \0 is *not* allowed, but honestly, I don't know. For the moment, I'm punting on that - most likely, the string will silently truncate at the \0.
Have also pushed some docs improvements, which were sorely needed (I don't need code review to tell me *that* :) )
Thanks for the eyeballing.
ChrisA
Marcus Comstedt on LysKOM:
IIUC the target is supposed to be a MIME-type, which means that non-ASCII is simply not allowed. So you could check the min and max of the string to make sure it only contains ASCII and no control characters (such as NUL).
Huh. Didn't know that; the docs I was looking at didn't say, and the example I had used "STRING". But poking around on the internet shows some support for the "should be a MIME type" notion, and it answers my unspoken question "how in the world do you synchronize across applications", which doesn't affect my current project.
ChrisA
The XDND specification can be found here:
https://www.freedesktop.org/wiki/Specifications/XDND/
and states that
| All data types are referred to by their corresponding X Atoms. The | atom names are the corresponding MIME types, in all lower case. (RFC's | for MIME: 2045, 2047, 2048, 2049) | | For text, the charset attribute can be appended to the MIME | type. (e.g. Japanese -> text/plain;charset="ISO-2022-JP") If the | charset attribute is not specified, it is assumed to be | ISO-8859-1. (new in version 4)
STRING is one of the valid _clipboard_ types (see https://tronche.com/gui/x/icccm/sec-2.html#s-2.6.2), which might be the source of the confusion?
Oops, good catch on the sub-array size check. And yeah, added the assertion that size_shift be zero; although I'm not sure whether I should be UTF-8 encoding (which may apply even if size_shift is nonzero) or handling non-ASCII strings in some other way.
IIUC the target is supposed to be a MIME-type, which means that non-ASCII is simply not allowed. So you could check the min and max of the string to make sure it only contains ASCII and no control characters (such as NUL).
From a very quick look: _decode_targets in gtkwidget.pre doesn't check
that the sub-array size is 3 before indexing it. And checking that ITEM(cur)[0].u.string->size_shift is 0 is easy to add. Are there any other restrictions on the string (e.g. is \0 allowed)? The new string range functions are handy for that.
pike-devel@lists.lysator.liu.se
-
Chris Angelico -
Marcus Comstedt (ACROSS) (Hail Ilpalazzo!) ＠ Pike (-) developers forum -
Martin Nilsson (Coppermist) ＠ Pike (-) developers forum