| Type | Bug | Status | built | Date | 10-Jan-2011 01:17 |
|---|---|---|---|---|---|
| Version | alpha 110 | Category | Host-Kit | Submitted by | Kaj |
| Platform | Linux x86 libc6 | Severity | minor | Priority | high |
| Summary | RL_GET_STRING computes wrong address from incompatible item size |
|---|---|
| Description |
If you give an index to RL_GET_STRING on a series of single bytes, it will return an address computed as if the items are double bytes. I'd guess RL_GET_STRING is currently hardcoded to always count in double bytes. |
| Example code |
length = - RL_GET_STRING(binary, index, (void **) &pointer); |
| Assigned to | n/a | Fixed in | alpha 111 | Last Update | 31-Jan-2011 01:04 |
|---|
| Comments | |
|---|---|
|
(0003014)
oldes 13-Jan-2011 23:10 |
Maybe we could have RL_GET_BINARY, couldn't we? See also http://issue.cc/r3/1823 |
|
(0003026)
Carl 17-Jan-2011 04:25 |
It's not hard coded. There must be a problem with how the string was created. Can you provide the full example? |
|
(0003027)
Kaj 17-Jan-2011 17:53 |
Here's the callback where RL_GET_STRING is used, with a workaround in place:
// Callback for filling the buffer for writing size_t read_buffer(char *buffer, size_t width, size_t chunks, void *session) { char *data; u32 tail, length = chunks * width; // R3 A110 FIXME: address miscomputed as if items are double bytes: // tail = - RL_GET_STRING(((struct session *) session)->output, ((struct session *) session)->progress, (void **) &data); tail = - RL_GET_STRING(((struct session *) session)->output, 0, (void **) &data) - ((struct session *) session)->progress; if (tail < length) length = tail; // memcpy(buffer, data, length); // Thread safe? memcpy(buffer, data + ((struct session *) session)->progress, length); ((struct session *) session)->progress += length; return length; } The series and session record are set up as follows: struct session { CURL *handle; CURLcode status; REBSER *input; REBSER *output; u64 progress; struct curl_slist *headers; // Progress callback REBSER *context; u32 word; }; ... case 7: { // write session = RXA_HANDLE(arguments, 1); handle = session->handle; if ((series = session->output) != NULL) RL_PROTECT_GC(series, 0); session->output = series = RXA_SERIES(arguments, 2); RL_PROTECT_GC(series, 1); session->progress = RXA_INDEX(arguments, 2); if (status = curl_easy_setopt(handle, CURLOPT_READDATA, session)) break; if (status = curl_easy_setopt(handle, CURLOPT_READFUNCTION, read_buffer)) break; ... |
|
(0003032)
Kaj 18-Jan-2011 23:41 |
The binary that was passed to the WRITE command where the corruption was observed came from READ FILE, so maybe the problem is in there. |
|
(0003034)
Kaj 18-Jan-2011 23:50 |
Here's the interface description of the command:
write: command [{Prepare write action.} session [handle!] data [binary!]] So we should really be getting a binary. The series is protected because the buffer is actually written in later command calls. No reference to it survives in the REBOL program, so it is not changed to a string inbetween or something like that. Also, with the workaround, the written files are correct, so they are really single byte. |
|
(0003046)
oldes 25-Jan-2011 20:45 |
Maybe to make it more clear...
srcLen = - RL_GET_STRING(ser, RXA_INDEX(frm, 1), (void **)&srcData); srcData -= RXA_INDEX(frm, 1); //<--- NOTE that you must correct the pointer RL_GET_STRING multiplies the second (index) argument by 2. |
|
(0003047)
oldes 25-Jan-2011 20:59 |
Also there may be problem the fact that when you create binary on rebol side, RXI_SER_WIDE is always set to 1.
... "export bin-test: command [bin [binary!]]\n" ... case 1: { char *srcData; u32 srcLen; REBSER *ser = RXA_SERIES(frm, 1); RL_PRINT("RXI_SER_WIDE=%d\n", RL_SERIES(ser, RXI_SER_WIDE)); RL_PRINT("RXI_SER_SIZE=%d\n", RL_SERIES(ser, RXI_SER_SIZE)); RL_PRINT("RXI_SER_LEFT=%d\n", RL_SERIES(ser, RXI_SER_LEFT)); srcLen = - RL_GET_STRING(ser, RXA_INDEX(frm, 1), (void **)&srcData); //srcData-= RXA_INDEX(frm, 1); RL_PRINT("index=%d\n", RXA_INDEX(frm, 1)); RL_PRINT("srcLen=%d\n", srcLen); return RXR_NONE; } ... bin-test #{78DAF348CDC9C90700058C01F5} bin-test next #{78DAF348CDC9C90700058C01F5} bin-test to-binary "hello" >> RXI_SER_WIDE=1 RXI_SER_SIZE=16 RXI_SER_LEFT=2 index=0 srcLen=13 RXI_SER_WIDE=1 RXI_SER_SIZE=16 RXI_SER_LEFT=2 index=1 srcLen=12 RXI_SER_WIDE=1 RXI_SER_SIZE=8 RXI_SER_LEFT=2 index=0 srcLen=5 |
|
(0003049)
abolka 25-Jan-2011 22:21 |
You probably should not use RL_GET_STRING to access the data of a binary!, but RL_SERIES(series, RXI_SER_DATA) instead. |
|
(0003062)
Carl 31-Jan-2011 01:04 |
The size check logic was reversed, so this function wasn't working correctly at all. It's fixed now in A111. |
| Date | User | Field | Action | Change |
|---|---|---|---|---|
| 31-Jan-2011 01:04 | carl | Comment : 0003062 | Added | - |
| 31-Jan-2011 01:02 | carl | Fixedin | Modified | => alpha 111 |
| 31-Jan-2011 01:02 | carl | Status | Modified | reviewed => built |
| 25-Jan-2011 22:21 | abolka | Comment : 0003049 | Added | - |
| 25-Jan-2011 21:02 | oldes | Comment : 0003047 | Modified | - |
| 25-Jan-2011 21:02 | oldes | Comment : 0003047 | Modified | - |
| 25-Jan-2011 20:59 | oldes | Comment : 0003047 | Added | - |
| 25-Jan-2011 20:45 | oldes | Comment : 0003046 | Added | - |
| 18-Jan-2011 23:50 | Kaj | Comment : 0003034 | Added | - |
| 18-Jan-2011 23:41 | Kaj | Comment : 0003032 | Added | - |
| 17-Jan-2011 17:53 | Kaj | Comment : 0003027 | Added | - |
| 17-Jan-2011 04:25 | carl | Comment : 0003026 | Added | - |
| 17-Jan-2011 04:24 | carl | Description | Modified | - |
| 17-Jan-2011 04:24 | carl | Status | Modified | submitted => reviewed |
| 13-Jan-2011 23:10 | oldes | Comment : 0003014 | Added | - |
| 10-Jan-2011 01:17 | Kaj | Ticket | Added | - |