On Tue, Oct 4, 2011 at 9:13 PM, Peter Hutterer <peter.hutte...@who-t.net> wrote:
> On Tue, Sep 27, 2011 at 06:03:55PM -0700, Jason Gerecke wrote:
>> + * @return Pointer to the requested data. Must be 'free()'-ed
>> + */
>> +static void* _get_property(Display *dpy, XDevice *dev, const char
>> *prop_name,
>> + int format, Atom type, unsigned long *items)
>
> this should better be nitems to avoid confusion between the actual items and
> the _number_ of items. also, I'd prefer if you return char* instead of
> void*.
> also, no real need for unsigned long here, might as well use int or uint.
>
I'm still too new to C to understand all the nuances, so I must ask --
why should char* be preferred over void* in this instance? I've seen
both used in situations like this, but it seems that void* is the more
correct type (mostly based on the observation that functions like
malloc, memset, and memcmp all use void* when the type cannot be
known).
As for the unsigned long, that's the type I get back from
XGetDeviceProperty -- switching to int and uint gives us nothing but
more compiler warnings :)
>> + }
>> + else
>> + {
>> + data = tmp;
>> + memcpy(&data[offset], read, read_items * format / 8);
>> + XFree(read);
>> + }
>> + }
>> + while (bytes_after > 0);
>
> the loop seems a bit excessive. just ask the server to fetch 1000 values or
> so and we're covered for what we support, right?
>
I'd tend to agree with you, but the loop is here out of principle.
Claiming "N should be enough" has always been a great way to get
yourself into trouble. I'd rather the code be as correct as possible,
even if it comes at the expense of some readability (though if you've
got a more readable way to handle the loop, I'm all ears!).
On Tue, Oct 4, 2011 at 9:19 PM, Peter Hutterer <peter.hutte...@who-t.net> wrote:
>
> actually, if you want to make this really useful, you could return a union
> in the form
>
> union propdata {
> char *c;
> int *i;
> float *f;
> };
>
> and do the convertion to/fro the stupid property long format for 32-byte
> properties. it would likely make the callers more clear too. see xinput's
> src/property.c for an example.
>
Just implemented this and I'm not really sure if it helps with
readability. If we know the correct type for the pointer (as is the
case at most call sites), it makes things more difficult to read. On
the other hand, when we don't know the correct type (like in
'get_param'), things are easier to read. I can post the patch with the
return type switched to propdata if you want to compare them.
Jason
---
Day xee-nee-svsh duu-'ushtlh-ts'it;
nuu-wee-ya' duu-xan' 'vm-nvshtlh-ts'it.
Huu-chan xuu naa~-gha.
------------------------------------------------------------------------------
All the data continuously generated in your IT infrastructure contains a
definitive record of customers, application performance, security
threats, fraudulent activity and more. Splunk takes this data and makes
sense of it. Business sense. IT sense. Common sense.
http://p.sf.net/sfu/splunk-d2d-oct
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
