Re: xhash and it's key
Dnia 2014-08-28, czw o godzinie 17:51 +, Shawn Debnath pisze: The problem is that it breaks scenarios where the user may use a temp buffer to build the key, then insert or put it in the xhash and then free the buffer memory. This is invalid use of xhash. Assumption here is that xhash code would allocate necessary buffer to store internal data and not rely on user supplied memory to maintain it=A9=F6s internal data structures. There is no such assumption. It's a gotcha waiting for every new jabberd2 dev. ;-) Any ideas if there was a particular reason this was designed this way? I imagine, in most of the cases the key is inside the object being stored so it works out. This is for efficiency reasons. Strings in jabberd are usually coming from incoming NADs (notice xhash_putx() taking the len of the key) or being allocated in memory pools associated with objects. It would be a waste of memory and CPU to make a copy each time an object gets stored in hash or removed from hash. Also, when these strings are part of the object they identify, memory management is as easy as freeing the object and it's associated memory pool (assuming it was already removed from all its references including xhashes). However, as you can see, the xhash implementation can¹t be fully exploited/used. The fact you are allocating object identifier strings on stack/heap is a sign of bad design. Could you rethink your design to include the identifier as a part of the object it names?
Re: xhash and it's key
Could you rethink your design to include the identifier as a part of the object it names? Yep, looks like this is a cleaner way out.
xhash and it's key
Hey there, Turns out that when inserting items in xhash, the code stores a pointer to the key passed in by the user in the xhash node and uses that later to compare in _get. The problem is that it breaks scenarios where the user may use a temp buffer to build the key, then insert or put it in the xhash and then free the buffer memory. Assumption here is that xhash code would allocate necessary buffer to store internal data and not rely on user supplied memory to maintain it=A9=F6s internal data structures. In the example above, the put succeeds, user frees the memory, so its Pointing to garbage and a later a call to _get can't find the entry because the keys mismatch, i.e., a user supplied valid string key and NULL memory that the xhash node points to. Any ideas if there was a particular reason this was designed this way? I imagine, in most of the cases the key is inside the object being stored so it works out. However, as you can see, the xhash implementation can¹t be fully exploited/used. It would be trivial, I would imagine, to make a copy of the key and store that instead of just the pointer to user supplied memory. On removal in zap_internal, the memory would be freed. Thoughts? Concerns? Let me know :) Shawn
Re: xhash and it's key
Pull request #79 submitted. https://github.com/jabberd2/jabberd2/pull/79 On 8/28/14, 10:51, Shawn Debnath sh...@debnath.net wrote: Hey there, Turns out that when inserting items in xhash, the code stores a pointer to the key passed in by the user in the xhash node and uses that later to compare in _get. The problem is that it breaks scenarios where the user may use a temp buffer to build the key, then insert or put it in the xhash and then free the buffer memory. Assumption here is that xhash code would allocate necessary buffer to store internal data and not rely on user supplied memory to maintain it=A9=F6s internal data structures. In the example above, the put succeeds, user frees the memory, so its Pointing to garbage and a later a call to _get can't find the entry because the keys mismatch, i.e., a user supplied valid string key and NULL memory that the xhash node points to. Any ideas if there was a particular reason this was designed this way? I imagine, in most of the cases the key is inside the object being stored so it works out. However, as you can see, the xhash implementation can¹t be fully exploited/used. It would be trivial, I would imagine, to make a copy of the key and store that instead of just the pointer to user supplied memory. On removal in zap_internal, the memory would be freed. Thoughts? Concerns? Let me know :) Shawn
