Code review comment for lp://qastaging/~3v1n0/unity/lim

On 1 February 2012 17:40, Marco Trevisan (Treviño) <mail@3v1n0.net> wrote:
>> 862     + if (sscanf (entry_id, "%p", &entry) == 1)
>> 863     + {
>> 864     + /* Checking that entry is really an IndicatorObjectEntry.
>> [...]
>>
>> Clever, but, why are we even checking for the validity of a pointer here ? Are
>> we passing pointers around in dbus ?
>
> Yes.
> That's what we've done so far. Actually they're just unique ID, but since they're pointers in fact we can just use these ids to quickly get the struct we're referring to, without using other data to store them that would increase the computation and the memory used (and considering the overhead that dbus already causes to us when scrubbing menus, the faster we are, the best it is). Since we can be more smart (and safe), using low-level things we use them.
>
> Also, since the IndicatorObjectEntry's are only structs and not GObject's, this is the only way we have to check if we're really working with a valid one.

There's no way to know that, and the pipe() trick does not ensure that
either. If the IndicatorObjectEntry is allocated with the slice
allocated (fx.) it'll still be valid memory even after freed, but
might contain garbage.

I am not entirely understanding the reasoning here. Is it that
creating a pipe and checking the write is cheaper than a hashtable
lookup? I doubt it as pipe(), write(), and close() requires a few mode
switches. And arguably I'd consider the hashtable trick more safe.

(although the pipe trick is neat, I am also wondering if there
wouldn't be some lower level call that checked this directly..? not
arguing in the favour of that though :-))