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

[José Fonseca]

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.

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