[lkml]   [2012]   [Jun]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH] perf symbols: Follow .gnu_debuglink section to find separate symbols
    Em Wed, Jun 20, 2012 at 03:22:41PM -0700, Pierre-Loup A. Griffais escreveu:
    > The .gnu_debuglink section is specified to contain the filename of the
    > debug info file, as well as a CRC that can be used to validate it.

    > This provides more context:
    > +++ b/tools/perf/util/symbol.c
    > +static int filename__read_debuglink(const char *filename,
    > + char *path, size_t size)
    > +{

    Isn't there any other function that opens an ELF file, looks for an
    specific session to then read its contents?

    Couldn't it be reused here?

    > + case SYMTAB__DEBUGLINK: {
    > + char *last_slash;
    > + strncpy(name, dso->long_name, size);
    > + last_slash = name + dso->long_name_len;
    > + while (last_slash && *last_slash != '/')
    > + last_slash--;

    Why the test for last_slash to be != NULL? How could it ever be? This is
    an optimization since we have the dso->long_name_len so that we avoid
    using strrchr that in turn would do an strlen?

    So the test should be:

    while (last_slash != name && *last_slash != '/')

    To avoid underflowing, right?

    > + if (last_slash)
    > + last_slash++;
    > + filename__read_debuglink(dso->long_name, last_slash,

    How last_slash can point to the path? It looks like it points to the
    basename, no?

    Yeah, it is, and then your algorithm will work because last_slash
    doesn't point to the _path_, but to a string _preceded_ by the path, so
    for /bin/ls, the debuglink content would be ls.debug and that is what
    will be stashed there, ending up with:

    name = "/bin/\0"
    name = "/bin/ls.debug\0"

    I.e. its not really the last slash, but where the debuglink content has
    to be stashed, concatenating with the same dirname as the associated

    Can you please rename "last_slash" to "debuglink"? And "path" to
    "debuglink" as well in the routine that reads the debuglink.

    > + size - (last_slash - name));
    > + }

    Other than that, thanks a lot for working on this, surely we have to
    support this feature!

    - ARnaldo

     \ /
      Last update: 2012-06-21 20:01    [W:0.024 / U:75.664 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site