[ 
https://issues.apache.org/jira/browse/THRIFT-3968?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Simon South updated THRIFT-3968:
--------------------------------
    Attachment: 0-thrift-3968-add-test-cases.patch

Having looked into this, I think the existing implementation is correct.

[The C++ implementation's {{readBinary}} 
method|https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=blob;f=lib/cpp/src/thrift/protocol/TProtocol.h;h=eb323b0cb0654ec0a121976fa9ea22b2634ba30a;hb=16a23a6618754a5a87aeb8df99a72516b0272fb3#l601]
 returns a {{std::string}}, while [the C (GLib) 
equivalent|https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=blob;f=lib/c_glib/src/thrift/c_glib/protocol/thrift_protocol.h;h=58fe5e0afa9ee27c0988c4c4ce20f73687e8aca0;hb=16a23a6618754a5a87aeb8df99a72516b0272fb3#l172]
 returns a pointer to a buffer in memory. While the concept of a zero-length 
string exists, there isn't really such a thing as a zero-length memory buffer, 
except one represented by a null pointer—which is what the current 
implementation returns.

Even explicitly allocating a zero-length buffer would achieve only the same 
behaviour. From [the GLib reference manual's entry on 
{{g_new}}|https://developer.gnome.org/glib/2.26/glib-Memory-Allocation.html#g-new]:

bq. Allocates n_structs elements of type struct_type... If n_structs is 0 it 
returns NULL.

So I think the existing implementation is correct, unless we want to go as far 
as reimplementing the binary-field methods to work with strings instead. I 
recommend we leave things as they are.

I've updated the first patch with test cases that illustrate explicitly the 
handling of zero-length binary fields, but otherwise I think the work here is 
complete. [~cjmay], any additional thoughts before I commit?

> Deserializing empty string/binary fields
> ----------------------------------------
>
>                 Key: THRIFT-3968
>                 URL: https://issues.apache.org/jira/browse/THRIFT-3968
>             Project: Thrift
>          Issue Type: Bug
>          Components: C glib - Compiler, C glib - Library
>    Affects Versions: 0.9.3, 1.0
>            Reporter: Matej Kupljen
>            Assignee: Simon South
>         Attachments: 0-thrift-3968-add-test-cases.patch, 
> 1-thrift-3968-deserialize-empty-string.patch
>
>
> If we use a string inside struct definition and set the value to "" (empty 
> string, not NULL) when de-serializing it we get set it NULL and and the same 
> time the __isset field for this value is set to TRUE which is think is not 
> correct. 
> Either we fix the library to return an empty string, or fix the compiler to 
> set the __isset filed to FALSE.
> What would be the best way to solve this?
> Please, let me know if you need any more information.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to