This is the mail archive of the elfutils-devel@sourceware.org mailing list for the elfutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Add C++ wrappers for iterating through various libdw and libdwfl objects


Hi,

Sorry for the late reply. I had hoped someone with a bit more C++ foo
would comment first. Apologies for some of the basic questions. Assume
my C++ foo is below zero.

On Wed, 2015-03-18 at 00:55 +0100, Petr Machata wrote:
> these iterators originated a couple years back on the dwarflint branch.
> Back then Roland added some C++ wrappers and we started writing stuff in
> C++.  I've been lugging them around ever since, first to dwlocstat,
> which was peeled off dwarflint (and should be coming for review after
> this bunch), and then to dwgrep.  They are header-only library.

When they are header-only how do we check ABI? I assume users might
expose something like cu_iterator or cu_info from their own code. Is
there an easy way to check we won't break ABI between elfutils releases
for such usage? How or against what would we run libabigail for example?

I do find it a little messy to be honest. It makes it a bit hard to
quickly get an overview of the interface exposed by the header because
you have to skip through all the implementation stuff.

> They are not completely zero-overhead, as every "++" involves a call to
> the underlying C-level interface.  But they are tremendously handy and
> allow one to express algorithms in a native C++ way, which is why I've
> been using them.

The examples in the header are very helpful. Thanks.

> Error conditions are communicated through exceptions.  For better or
> worse, that's how things are done in C++, and there's simply no way to
> communicate failures out of constructors or the likes of operator++.
> Currently I'm simply throwing an std::runtime_error, but maybe it would
> make sense to have a custom family of libelf, libdw, libdwfl exceptions,
> all rooted in a general elfutils exception type.

Can we add those later, inheriting from std::runtime_error for example
to keep ABI?

> One non-iterator-like concept that I included as well is a wrapper
> around dwfl_report.  It's an RAII class that ends the reporting when it
> goes out of scope.  As soon as one calls dwfl_report_begin, they assume
> responsibility to call dwfl_report_end eventually.  But in C++ many
> error conditions are communicated through exceptions, and making sure
> these cleanups are not skipped is messy.  RAII as a way to express these
> responsibilities in C++.

The example here can be extended a little. It isn't immediately clear
what the advantage is from the two lines. It feels like a wrapper for
creating a Dwfl is missing. But maybe it is natural to mix-and-match the
C libdwfl.h and c++/libdwfl calls. Expanding the example might show why
it is really cool. Maybe one that wraps everything in a function or
block to show the RAII and cleanup triggering?

> It's all C++98.  We have had "the new C++" out there on the streets
> since 2009 or so, and the iterators presented here were all written with
> varying degrees of C++11 in them.  But that is still sufficiently new
> that I felt adhering to the C++98 subset is the prudent way forward, and
> backported the code.

Are there any issues building this with C++11? Does that change ABI?

> The headers are installed unconditionally, but a test for availability
> of a C++ compiler is made anyway, and build of the corresponding test is
> skipped if there is none.  The testing is rather light, including only a
> couple cases that I wanted to make sure about as I reviewed the code.
> Dwgrep has a fairly comprehensive test suite that covers proper function
> of these iterators, if that's any consolation.

I think I would prefer to only install them when we are able to test
them. And if people don't want them we could have a --disable-c++
configure option that skips them. Should packagers be encouraged to
package them in a separate elfutils-c++-devel package?

> +namespace elfutils
> +{
> +  // Helper functions.  Not for client consumption.
> +  namespace libdw_impl
> +  {
> +    inline void throw_libdw (int dwerr = 0);
> +    inline bool dwpp_child (Dwarf_Die &die, Dwarf_Die &result);
> +    inline bool dwpp_siblingof (Dwarf_Die &die, Dwarf_Die &result);
> +    inline Dwarf_Die dwpp_offdie (Dwarf *dbg, Dwarf_Off offset);
> +  }

So this is just a convention?
Any namespace ending in _impl isn't really part of the API?

> +  // The iterators presented here have operators * and -> as non-const
> +  // member functions.  The reason is that the libdw interfaces do not
> +  // take, say, Dwarf_Die const *, but Dwarf_Die *, and therefore
> +  // returning a const reference of some sort would not be useful.  We
> +  // don't want to give out copies either, as that adds unnecessary
> +  // overhead.  And we simply don't care much if anyone does end up
> +  // changing the internal copy of the current CU, DIE, or whatever.
> +
> +
> +  // An iterator that goes over compile units (full or partial) in a
> +  // given DWARF file.  The type that it points to (yields on
> +  // dereference) is CU_INFO (see below).
> +  //
> +  // Example usage:
> +  // {
> +  //   std::vector <Dwarf_Off> v
> +  //   for (elfutils::cu_iterator jt (dw);
> +  //        jt != elfutils::cu_iterator::end (); ++jt)
> +  //     v.push_back (dwarf_dieoffset (&jt->cudie));
> +  // }
> +  class cu_iterator;
> +
> +  // Helper structure with data about each compile unit.
> +  struct cu_info
> +  {
> +    Dwarf_Die cudie;
> +    Dwarf_Off abbrev_offset;
> +    uint8_t address_size;
> +    uint8_t offset_size;
> +  };

This is all useful information that we get from dwarf_nextcu () anyway,
so it is cheap to provide. But don't we also want to provide any type
unit information? I see the code uses dwarf_nextcu () and dwarf_offdie
() so it cannot really handle type units (as long as they are in
separate sections, DWARF5 proposes to merge them into the
main .debug_info instead of the separate .debug_type). Was it a
deliberate choice to ignore type units?

> +  class cu_iterator
> +    : public std::iterator <std::input_iterator_tag, cu_info>
> +  {
> +    friend class die_tree_iterator;
> +    Dwarf *m_dw;
> +    Dwarf_Off m_offset;
> +    Dwarf_Off m_old_offset;
> +    cu_info m_info;
> +
> +    explicit cu_iterator (Dwarf_Off off)
> +      : m_dw (NULL)
> +      , m_offset (off)
> +      , m_old_offset (0)
> +    {}

A NULL m_dw seems dangerous/sloppy.

> +    void
> +    move ()
> +    {
> +      assert (*this != end ());
> +      m_old_offset = m_offset;
> +      size_t hsize;
> +      if (dwarf_nextcu (m_dw, m_offset, &m_offset, &hsize,
> +			&m_info.abbrev_offset,
> +			&m_info.address_size, &m_info.offset_size) != 0)
> +	done ();
> +      else
> +	m_info.cudie
> +	  = elfutils::libdw_impl::dwpp_offdie (m_dw, m_old_offset + hsize);
> +    }
> +
> +    void
> +    done ()
> +    {
> +      *this = end ();
> +    }
> +
> +  public:
> +    explicit cu_iterator (Dwarf *dw)
> +      : m_dw (dw)
> +      , m_offset (0)
> +      , m_old_offset (0)
> +    {
> +      move ();
> +    }

Just a simple c++ question. What kind of stuff would convert as argument
if we don't use explicit here?

> +    // Construct a CU iterator for DW such that it points to a compile
> +    // unit represented by CUDIE.
> +    cu_iterator (Dwarf *dw, Dwarf_Die cudie)
> +      : m_dw (dw)
> +      , m_offset (dwarf_dieoffset (&cudie) - dwarf_cuoffset (&cudie))
> +      , m_old_offset (0)
> +    {
> +      move ();
> +    }

And why don't we need explicit here?

> +    cu_iterator (cu_iterator const &that)
> +      : m_dw (that.m_dw)
> +      , m_offset (that.m_offset)
> +      , m_old_offset (that.m_old_offset)
> +      , m_info (that.m_info)
> +    {}
> +
> +    // Return a cu_iterator pointing one after the last actual CU.
> +    static cu_iterator
> +    end ()
> +    {
> +      return cu_iterator ((Dwarf_Off) -1);
> +    }
> +
> +    bool
> +    operator== (cu_iterator const &that) const
> +    {
> +      return m_offset == that.m_offset;
> +    }

Don't you also need to compare m_dw?
hmmm, that doesn't work with end () then.

> +    bool
> +    operator!= (cu_iterator const &that) const
> +    {
> +      return ! (*this == that);
> +    }
> +
> +    cu_iterator
> +    operator++ ()
> +    {
> +      move ();
> +      return *this;
> +    }
> +
> +    cu_iterator
> +    operator++ (int)
> +    {
> +      cu_iterator tmp = *this;
> +      ++*this;
> +      return tmp;
> +    }

This is just idiom for implementing it++?
While the above is the ++it implementation?
(I don't understand the (int))

> +    Dwarf_Off
> +    offset () const
> +    {
> +      return m_old_offset;
> +    }

Why do we explicitly expose this? Is this more convenient than having it
as part of cu_info? And does the user really care about this offset? It
could also just be queried with dwarf_cu_die for example.

> +    // N.B. see top of the file for explanation of non-constness of
> +    // operators * and ->.

That holds for Dwarf_Die. But can't/shouldn't we return a const cu_info?
Or does that not make sense if some fields are non-const anyway?

> +    cu_info &
> +    operator* ()
> +    {
> +      return m_info;
> +    }
> +
> +    cu_info *
> +    operator-> ()
> +    {
> +      return &**this;
> +    }
> +  };
> +
> +  // An iterator that goes through children of a given DIE.
> +  // Example usage:
> +  // {
> +  //    size_t nchildren = std::distance (elfutils::child_iterator (type_die),
> +  //                                      elfutils::child_iterator::end ());
> +  // }

Cute :)

> +  class child_iterator
> +    : public std::iterator <std::input_iterator_tag, Dwarf_Die>
> +  {
> +    Dwarf_Die m_die;
> +
> +    child_iterator ()
> +      : m_die ({.addr = (void *) -1})
> +    {}
> +
> +  public:
> +    explicit child_iterator (Dwarf_Die parent)
> +    {
> +      if (! elfutils::libdw_impl::dwpp_child (parent, m_die))
> +	*this = end ();
> +    }
> +
> +    child_iterator (child_iterator const &that)
> +      : m_die (that.m_die)
> +    {}
> +
> +    // Return a child_iterator pointing one after the last actual
> +    // child.
> +    static child_iterator
> +    end ()
> +    {
> +      return child_iterator ();
> +    }
> +
> +    bool
> +    operator== (child_iterator const &that) const
> +    {
> +      return m_die.addr == that.m_die.addr;
> +    }
> +
> +    bool
> +    operator!= (child_iterator const &that) const
> +    {
> +      return ! (*this == that);
> +    }
> +
> +    child_iterator
> +    operator++ ()
> +    {
> +      assert (*this != end ());
> +      if (! elfutils::libdw_impl::dwpp_siblingof (m_die, m_die))
> +	*this = end ();
> +      return *this;
> +    }
> +
> +    child_iterator
> +    operator++ (int)
> +    {
> +      child_iterator ret = *this;
> +      ++*this;
> +      return ret;
> +    }
> +    // N.B. see top of the file for explanation of non-constness of
> +    // operators * and ->.
> +
> +    Dwarf_Die &
> +    operator* ()
> +    {
> +      assert (*this != end ());
> +      return m_die;
> +    }

Is assert the best idiom to use here?
Isn't there a runtime exception for this?

> +    Dwarf_Die *
> +    operator-> ()
> +    {
> +      return &**this;
> +    }
> +  };

Just thinking of naming here. I assume we would also want to provide a
child iterator that transparently handles import_unit/partial_units.
Given that often that is the one people might want to use instead of
having to deal with "raw" DIE trees. How would we call that one?

> +  // Tree flattening iterator.  It pre-order iterates all DIEs in
> +  // given DWARF file, optionally starting from a given CU iterator.
> +  // It keeps track of path from CU root to the current DIE, and that
> +  // can be requested through stack() member function.
> +  //
> +  // Example usage:
> +  // {
> +  //   for (elfutils::die_tree_iterator it (dw);
> +  //        it != elfutils::die_tree_iterator::end (); ++it)
> +  //     {
> +  //       typedef elfutils::die_tree_iterator::stack_type stack_type;
> +  //       stack_type const &stack = it.stack ();
> +  //       for (stack_type::const_iterator jt = stack.begin ();
> +  //            jt != stack.end (); ++jt)
> +  //         ...;
> +  //     }
> +  // }
> +  class die_tree_iterator
> +    : public std::iterator <std::input_iterator_tag, Dwarf_Die>
> +  {
> +  public:
> +    typedef std::vector <Dwarf_Die> stack_type;
> +
> +  private:
> +    cu_iterator m_cuit;
> +    // Internally, only offsets are kept on the stack.
> +    std::vector <Dwarf_Off> m_stack;
> +    Dwarf_Die m_die;
> +
> +    die_tree_iterator (Dwarf_Off offset)
> +      : m_cuit (cu_iterator::end ())
> +    {}
> +
> +  public:
> +    explicit die_tree_iterator (Dwarf *dw)
> +      : die_tree_iterator (cu_iterator (dw))
> +    {}
> +
> +    explicit die_tree_iterator (cu_iterator const &cuit)
> +      : m_cuit (cuit)
> +      , m_die (m_cuit->cudie)
> +    {}
> +
> +    die_tree_iterator (die_tree_iterator const &that)
> +      : m_cuit (that.m_cuit)
> +      , m_stack (that.m_stack)
> +      , m_die (that.m_die)
> +    {}

No way to walk the type units?

> +    // Return a die_tree_iterator pointing one after the last actual
> +    // DIE.
> +    static die_tree_iterator
> +    end ()
> +    {
> +      return die_tree_iterator ((Dwarf_Off) -1);
> +    }
> +
> +    bool
> +    operator== (die_tree_iterator const &that) const
> +    {
> +      return m_cuit == that.m_cuit
> +	&& m_stack == that.m_stack
> +	&& (m_cuit == cu_iterator::end ()
> +	    || m_die.addr == that.m_die.addr);
> +    }
> +
> +    bool
> +    operator!= (die_tree_iterator const &that) const
> +    {
> +      return ! (*this == that);
> +    }
> +
> +    die_tree_iterator
> +    operator++ ()
> +    {
> +      Dwarf_Die child;
> +      if (elfutils::libdw_impl::dwpp_child (m_die, child))
> +	{
> +	  m_stack.push_back (dwarf_dieoffset (&m_die));
> +	  m_die = child;
> +	  return *this;
> +	}
> +
> +      do
> +	if (elfutils::libdw_impl::dwpp_siblingof (m_die, m_die))
> +	  return *this;
> +	else
> +	  // No sibling found.  Go a level up and retry, unless this
> +	  // was a sole, childless CU DIE.
> +	  if (! m_stack.empty ())
> +	    {
> +	      m_die = elfutils::libdw_impl::dwpp_offdie (m_cuit.m_dw,
> +							 m_stack.back ());
> +	      m_stack.pop_back ();
> +	    }
> +      while (! m_stack.empty ());
> +
> +      m_die = (++m_cuit)->cudie;
> +      return *this;
> +    }

If we had a "logical cu child iterator" would there be a way to extend
this to use either the logical child or low level DIE tree child
iterator instead of direct dwpp_siblingof/dwpp_offdie calls?

> +    die_tree_iterator
> +    operator++ (int)
> +    {
> +      die_tree_iterator prev = *this;
> +      ++*this;
> +      return prev;
> +    }

And again I don't understand. Sorry.

> +    // N.B. see top of the file for explanation of non-constness of
> +    // operators * and ->.
> +
> +    Dwarf_Die &
> +    operator* ()
> +    {
> +      return m_die;
> +    }
> +
> +    Dwarf_Die *
> +    operator-> ()
> +    {
> +      return &**this;
> +    }
> +
> +    // Return a stack of DIE's representing the path from CU DIE to
> +    // the current DIE (both ends inclusive).  The first element of
> +    // the returned stack is the CU DIE, the last one the current DIE.
> +    stack_type
> +    stack () const
> +    {
> +      stack_type ret;
> +      for (die_tree_iterator it = *this; it != end (); it = it.parent ())
> +	ret.push_back (*it);
> +      std::reverse (ret.begin (), ret.end ());
> +      return ret;
> +    }
> +
> +    // Return a die_tree_iterator pointing at a parent of a DIE that
> +    // this iterator points at.  Returns an end iterator if there is
> +    // no parent.
> +    die_tree_iterator
> +    parent () const
> +    {
> +      assert (*this != end ());
> +      if (m_stack.empty ())
> +	return end ();
> +
> +      die_tree_iterator ret = *this;
> +      ret.m_die = elfutils::libdw_impl::dwpp_offdie (m_cuit.m_dw,
> +						     m_stack.back ());
> +      ret.m_stack.pop_back ();
> +      return ret;
> +    }
> +
> +    // Return a CU iterator representing a CU that the current DIE
> +    // comes from.
> +    cu_iterator
> +    cu () const
> +    {
> +      return m_cuit;
> +    }

This interface might make it hard to also support "logical tree walking"
if we want to also handle import_unit/partial_unit. Or we could just say
"this always returns the low-level DIE tree CU".

> +  };
> +
> +  // An attribute iterator goes through attributes of a given DIE.
> +  class attr_iterator
> +    : public std::iterator <std::input_iterator_tag, Dwarf_Attribute>
> +  {
> +    Dwarf_Die *m_die;
> +    Dwarf_Attribute m_at;
> +    ptrdiff_t m_offset;
> +
> +    struct cb_data
> +    {
> +      Dwarf_Attribute *at;
> +      bool been;
> +    };

It would be nice to document these fields/structures a little to make
reading the implementation easier for noobs like me.

> +    static int
> +    callback (Dwarf_Attribute *at, void *data)
> +    {
> +      cb_data *d = static_cast <cb_data *> (data);
> +      if (d->been)
> +	return DWARF_CB_ABORT;
> +
> +      *d->at = *at;
> +      d->been = true;
> +
> +      // Do a second iteration to find the next offset.
> +      return DWARF_CB_OK;
> +    }
> +
> +    void
> +    move ()
> +    {
> +      // If m_offset is already 1, we are done iterating.
> +      if (m_offset == 1)
> +	{
> +	  *this = end ();
> +	  return;
> +	}
> +
> +      cb_data data = {&m_at, false};
> +      m_offset = dwarf_getattrs (m_die, &callback, &data, m_offset);
> +      if (m_offset == -1)
> +	elfutils::libdw_impl::throw_libdw ();
> +    }
> +
> +    attr_iterator (ptrdiff_t offset)
> +      : m_die (NULL)
> +      , m_at ({0})
> +      , m_offset (offset)
> +    {}
> +
> +  public:
> +    attr_iterator (Dwarf_Die *die)
> +      : m_die (die)
> +      , m_at ({0})
> +      , m_offset (0)
> +    {
> +      move ();
> +    }
> +
> +    bool
> +    operator== (attr_iterator const &other) const
> +    {
> +      return m_offset == other.m_offset
> +	&& m_at.code == other.m_at.code;
> +    }

Don't you need to compare m_die too?

> +    bool
> +    operator!= (attr_iterator const &other) const
> +    {
> +      return ! (*this == other);
> +    }
> +
> +    attr_iterator &
> +    operator++ ()
> +    {
> +      assert (*this != end ());
> +      move ();
> +      return *this;
> +    }

Same question about assert as above.

> +    attr_iterator
> +    operator++ (int)
> +    {
> +      attr_iterator tmp = *this;
> +      ++*this;
> +      return tmp;
> +    }
> +
> +    // N.B. see top of the file for explanation of non-constness of
> +    // operators * and ->.
> +
> +    Dwarf_Attribute &
> +    operator* ()
> +    {
> +      return m_at;
> +    }
> +
> +    Dwarf_Attribute *
> +    operator-> ()
> +    {
> +      return &**this;
> +    }
> +
> +    // Return an attr_iterator pointing one after the last actual
> +    // attribute.
> +    static attr_iterator
> +    end ()
> +    {
> +      return attr_iterator ((ptrdiff_t) 1);
> +    }
> +  };
> +}
> +
> +inline void
> +elfutils::libdw_impl::throw_libdw (int dwerr)
> +{
> +  if (dwerr == 0)
> +    dwerr = dwarf_errno ();
> +  assert (dwerr != 0);
> +  throw std::runtime_error (dwarf_errmsg (dwerr));
> +}

This seems expensive (it copies the string doesn't it?).
Which might suggest having an explicit dw_error might be nicer.
Then one can call (or not call) dwarf_errmsg when catching.

> +inline bool
> +elfutils::libdw_impl::dwpp_child (Dwarf_Die &die, Dwarf_Die &result)
> +{
> +  int ret = dwarf_child (&die, &result);
> +  if (ret < 0)
> +    elfutils::libdw_impl::throw_libdw ();
> +  return ret == 0;
> +}
> +
> +inline bool
> +elfutils::libdw_impl::dwpp_siblingof (Dwarf_Die &die, Dwarf_Die &result)
> +{
> +  switch (dwarf_siblingof (&die, &result))
> +    {
> +    case -1:
> +      elfutils::libdw_impl::throw_libdw ();
> +    case 0:
> +      return true;
> +    case 1:
> +      return false;
> +    default:
> +      abort ();
> +    }
> +}
> +
> +inline Dwarf_Die
> +elfutils::libdw_impl::dwpp_offdie (Dwarf *dbg, Dwarf_Off offset)
> +{
> +  Dwarf_Die result;
> +  if (dwarf_offdie (dbg, offset, &result) == NULL)
> +    throw_libdw ();
> +  return result;
> +}
> +
> +#endif
> diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
> index d40dbae..45772b1 100644
> --- a/libdwfl/ChangeLog
> +++ b/libdwfl/ChangeLog
> @@ -1,3 +1,9 @@
> +2015-03-17  Petr Machata  <pmachata@redhat.com>
> +
> +	* c++: New directory.
> +	* c++/libdwfl: New header file.
> +	* Makefile.am: Install it.
> +
>  2015-01-26  Mark Wielaard  <mjw@redhat.com>
>  
>  	* dwfl_module_getdwarf.c (find_symtab): Explicitly clear symdata,
> diff --git a/libdwfl/Makefile.am b/libdwfl/Makefile.am
> index 72c980b..8d4a0f1 100644
> --- a/libdwfl/Makefile.am
> +++ b/libdwfl/Makefile.am
> @@ -2,7 +2,7 @@
>  ##
>  ## Process this file with automake to create Makefile.in
>  ##
> -## Copyright (C) 2005-2010, 2013 Red Hat, Inc.
> +## Copyright (C) 2005-2010, 2013, 2015 Red Hat, Inc.
>  ## This file is part of elfutils.
>  ##
>  ## This file is free software; you can redistribute it and/or modify
> @@ -39,6 +39,9 @@ noinst_LIBRARIES += libdwfl_pic.a
>  
>  pkginclude_HEADERS = libdwfl.h
>  
> +cppdir = $(pkgincludedir)/c++
> +cpp_HEADERS = c++/libdwfl

Is installing in a c++ subdir common?
Why not in the main include dir?
The file names don't conflict.

>  libdwfl_a_SOURCES = dwfl_begin.c dwfl_end.c dwfl_error.c dwfl_version.c \
>  		    dwfl_module.c dwfl_report_elf.c relocate.c \
>  		    dwfl_module_build_id.c dwfl_module_report_build_id.c \
> diff --git a/libdwfl/c++/libdwfl b/libdwfl/c++/libdwfl
> new file mode 100644
> index 0000000..9243f14
> --- /dev/null
> +++ b/libdwfl/c++/libdwfl
> @@ -0,0 +1,227 @@
> +/* -*-c++-*-
> +   Copyright (C) 2009, 2010, 2011, 2012, 2014, 2015 Red Hat, Inc.
> +   This file is part of elfutils.
> +
> +   This file is free software; you can redistribute it and/or modify
> +   it under the terms of either
> +
> +     * the GNU Lesser General Public License as published by the Free
> +       Software Foundation; either version 3 of the License, or (at
> +       your option) any later version
> +
> +   or
> +
> +     * the GNU General Public License as published by the Free
> +       Software Foundation; either version 2 of the License, or (at
> +       your option) any later version
> +
> +   or both in parallel, as here.
> +
> +   elfutils is distributed in the hope that it will be useful, but
> +   WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   General Public License for more details.
> +
> +   You should have received copies of the GNU General Public License and
> +   the GNU Lesser General Public License along with this program.  If
> +   not, see <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef _LIBDWFL_CPP
> +#define _LIBDWFL_CPP	1
> +
> +#include <stdexcept>
> +#include <cassert>
> +#include <string>
> +#include <elfutils/libdwfl.h>
> +
> +namespace elfutils
> +{
> +  // Helper functions.  Not for client consumption.
> +  namespace libdwfl_impl
> +  {
> +    inline void throw_libdwfl (int dwerr = 0);
> +  }
> +
> +  // Given a Dwfl, iterates through its Dwfl_Module's.
> +  //
> +  // Example usage:
> +  // std::vector <Dwfl_Module *> mods (elfutils::dwfl_module_iterator (dwfl),
> +  //				       elfutils::dwfl_module_iterator::end ());
> +
> +  class dwfl_module_iterator
> +    : public std::iterator <std::input_iterator_tag, Dwfl_Module *>
> +  {
> +    Dwfl *m_dwfl;
> +    ptrdiff_t m_offset;
> +    Dwfl_Module *m_module;
> +
> +    static int
> +    module_cb (Dwfl_Module *mod, void **data, const char *name,
> +	       Dwarf_Addr addr, void *arg)
> +    {
> +      elfutils::dwfl_module_iterator *self
> +	= static_cast <elfutils::dwfl_module_iterator *> (arg);
> +      self->m_module = mod;
> +      return DWARF_CB_ABORT;
> +    }
> +
> +    void
> +    move ()
> +    {
> +      m_offset = dwfl_getmodules (m_dwfl, module_cb, this, m_offset);
> +      if (m_offset == -1)
> +	elfutils::libdwfl_impl::throw_libdwfl ();
> +    }
> +
> +    explicit dwfl_module_iterator (ptrdiff_t off)
> +      : m_dwfl (NULL)
> +      , m_offset (off)
> +    {}
> +
> +  public:
> +    dwfl_module_iterator (Dwfl *dwfl)
> +      : m_dwfl (dwfl)
> +      , m_offset (0)
> +    {
> +      move ();
> +    }
> +
> +    dwfl_module_iterator (const dwfl_module_iterator &that)
> +      : m_dwfl (that.m_dwfl)
> +      , m_offset (that.m_offset)
> +      , m_module (that.m_module)
> +    {}
> +
> +    static dwfl_module_iterator
> +    end ()
> +    {
> +      return dwfl_module_iterator ((ptrdiff_t) 0);
> +    }
> +
> +    dwfl_module_iterator &
> +    operator++ ()
> +    {
> +      assert (*this != end ());
> +      move ();
> +      return *this;
> +    }
> +
> +    dwfl_module_iterator
> +    operator++ (int)
> +    {
> +      dwfl_module_iterator ret = *this;
> +      ++*this;
> +      return ret;
> +    }
> +
> +    Dwfl_Module &
> +    operator* () const
> +    {
> +      return *m_module;
> +    }
> +
> +    Dwfl_Module *
> +    operator-> () const
> +    {
> +      return &**this;
> +    }
> +
> +    bool
> +    operator== (const dwfl_module_iterator &that) const
> +    {
> +      assert (m_dwfl == NULL || that.m_dwfl == NULL || m_dwfl == that.m_dwfl);
> +      return m_offset == that.m_offset;
> +    }

Why that assert? Why not include it in the check?

> +    bool
> +    operator!= (const dwfl_module_iterator &that) const
> +    {
> +      return !(*this == that);
> +    }
> +  };
> +
> +
> +  // RAII-style wrapper around dwfl_report_* calls.
> +  //
> +  // Example usage:
> +  // {
> +  //   elfutils::dwfl_report r (dwfl);
> +  //   r.report_offline (fn, fn, fd);
> +  // }
> +
> +  class dwfl_report
> +  {
> +    Dwfl *m_dwfl;
> +
> +    Dwfl_Module *
> +    reported (Dwfl_Module *mod)
> +    {
> +      if (mod == NULL)
> +	elfutils::libdwfl_impl::throw_libdwfl ();
> +      return mod;
> +    }
> +
> +  public:
> +    explicit dwfl_report (Dwfl *dwfl)
> +      : m_dwfl ((assert (dwfl != NULL), dwfl))
> +    {
> +      dwfl_report_begin_add (m_dwfl);
> +    }

Is assert the best idiom for a NULL pointer check?

> +    ~dwfl_report ()
> +    {
> +      dwfl_report_end (m_dwfl, NULL, NULL);
> +    }
> +
> +    Dwfl_Module *
> +    report_module (const char *name, Dwarf_Addr start, Dwarf_Addr end)
> +    {
> +      return reported (dwfl_report_module (m_dwfl, name, start, end));
> +    }
> +
> +    Dwfl_Module *
> +    report_module (const std::string &name, Dwarf_Addr start, Dwarf_Addr end)
> +    {
> +      return report_module (name.c_str (), start, end);
> +    }
> +
> +    Dwfl_Module *
> +    report_elf (const char *name, const char *file_name, int fd,
> +		GElf_Addr base, bool add_p_vaddr)
> +    {
> +      return reported (dwfl_report_elf (m_dwfl, name, file_name, fd,
> +					base, add_p_vaddr));
> +    }
> +
> +    Dwfl_Module *
> +    report_elf (const std::string &name, const std::string &file_name, int fd,
> +		GElf_Addr base, bool add_p_vaddr)
> +    {
> +      return report_elf (name.c_str (), file_name.c_str (), fd,
> +			 base, add_p_vaddr);
> +    }
> +
> +    Dwfl_Module *
> +    report_offline (const char *name, const char *file_name, int fd)
> +    {
> +      return reported (dwfl_report_offline (m_dwfl, name, file_name, fd));
> +    }
> +
> +    Dwfl_Module *
> +    report_offline (const std::string &name, const std::string &file_name, int fd)
> +    {
> +      return report_offline (name.c_str (), file_name.c_str (), fd);
> +    }
> +  };
> +}
> +
> +inline void
> +elfutils::libdwfl_impl::throw_libdwfl (int dwerr)
> +{
> +  if (dwerr == 0)
> +    dwerr = dwfl_errno ();
> +  assert (dwerr != 0);
> +  throw std::runtime_error (dwfl_errmsg (dwerr));
> +}
> +
> +#endif

Thanks,

Mark

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]