[PATCH] egltrace/android: Fix tracing Zygote processes (v2)

[José Fonseca]

On Mon, Jun 17, 2013 at 6:48 PM, José Fonseca <jose.r.fonseca at gmail.com>wrote:

> On Mon, Jun 17, 2013 at 6:17 PM, Alexander Monakov <amonakov at ispras.ru>wrote:
>
>> > It would be nice to have an assert to truncate(ssize_t len) method that
>> > `find('\0') < buffer[len`], to catch similar cases to creep in the
>> future.
>> >
>> > Could you send me an updated patch?
>>
>> In the meantime I've sent the patch with the readlink error fixed in a
>> different way to the list.  Now truncate(len) is not used anywhere.
>>
>
> I see no point in calling strlen() when we already know the length
> beforehand.
>
> >  Please add `assert(strlen(str()) == length);` in the end of
> truncate(size_t) in a separate commit if you want.
>
> Will do.
>
> Overall I find this os::String class rather confusing to new and future
>> apitrace developers.  Are we supposed to use it for all strings, or
>> should we
>> use std::string where possible?
>
>
> Use std::string where possible.  The os::String's class comment is terse
> but clear (though misspelled):
>
>   /**
>    * Vector based zero-terminate string, suitable for passing strings or
> paths
>    * to/from OS calls.
>    */
>
> The crucial issue here is that both Win32 and POSIX return strings as zero
> length buffers, but std::string only provides read-only C strings. So there
> is no way to get the output of OS calls into a std::strings without using
> temporary mallocs, which seemed awful to me, especially because ultimately
> the strings would be passed back to OS which would again expect a zero
> terminator.
>
> I googled on this issue and the general advice online seemed to be that if
> you need to manipulate zero terminated strings then you're better off using
> std::vector<char> instead std::string, hence os::String was born.
>
> I suppose additional comments don't hurt, so I'll add these paragraphs as
> well.
>
>  Why was os::Path name decided less suitable?
>>
>
> Process names are not stricly paths, and I expected I need to use it for
> things like arguments, environment variables, etc -- most OS calls that
> return strings, even those that don't deal with paths, usually expect a
> sized buffer where to write it into.
>
>
>> And this truncate() method is a bit odd.
>>
>
> I'm not sure what you mean. The only odd thing I can see is the absence of
> a comment, but I'll fix that.
>
>
Don't know if this is what you meant, but indeed the str() call in
`truncate(strlen(str())) ` indeed is not correct, as the assertion inside
str() may fail erroneously.

Jose
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/apitrace/attachments/20130617/c3ce04f6/attachment-0001.html>