ChangeProperty not properly dealing with non-8-bit formats

[GoogleCodeExporter]

What steps will reproduce the problem?
Use ChangeProperty e.g. with 32 bit format

What is the expected output? What do you see instead?
expected: sets property correctly.
observed: dumps core

What version of the product are you using? On what operating system?
linux, revision 7

Please provide any additional information below.
The attached patch fixes the issue.

Original issue reported on code.google.com by a...@users.sf.net on 2 May 2010 at 4:00

Attachments:

• ChangeProperty.patch

[GoogleCodeExporter]

go_client.py handles all the []byte parameters in a uniform way.
Instead of modifying go_client.py to fix ChangeProperty, I prefer to move 
ChangeProperty 
from xproto.go to xgb.go and fix it there.

Original comment by frm.adip...@gmail.com on 3 May 2010 at 5:52

[GoogleCodeExporter]

Looking at the xproto.xml I am wondering if the way to 'go' would be to 
suppress the
format parameter from ChangeProperty alltogether:
There could be an interface for the data and both length and format could be
calculated from the data parameter and its type. Systematically for all lists of
void, I mean. The advantages are nobody has to manually reimplement parts of the
generated code and programs using the functions don't have to bother 
transforming
atoms (or lists thereof) to network byte order.
What do you think?
p.s.: Again, my python isn't really fluent enough to cleanly implement 
something like
this, so please excuse the missing patch.

Original comment by a...@users.sf.net on 3 May 2010 at 10:16

[GoogleCodeExporter]

I've attached a patch that fixes this in go_client.py, but not in an ideal way.

I think the problem here is the assumption that the "data_len" field in the 
ChangeProperty request corresponds to bytes. It doesn't. It corresponds to 
*format* units. Thus, with an 8 bit format, 1 byte corresponds to a length of 
1. Conversely, with a 32 bit format, 4 bytes corresponds to a length of 1.

In most other fields that end with "Len" (which is how go_client.py detects 
these kinds of fields), like the "name_len" field in InternAtom, it corresponds 
to bytes.

Thus, ChangeProperty either needs special attention or an additional parameter 
that lets the user define the "data_len" field. I've implemented the former in 
go_client.py, while xpyb (xgb's sibling project in Python) solves this with the 
latter approach. I chose the former because it keeps a consistent interface. 
(i.e., the illusion that "_len" fields are automatically computed.)

Original comment by jamslam@gmail.com on 25 Feb 2012 at 1:54

Attachments:

• changeProp.patch