Re: [PATCH 04/19] libctf: low-level list manipulation and helper utilities

[Nick Clifton <nickc at redhat dot com>]

Hi Nick,

> The distinction betweern this file and ctf-subr.c is lost in the mists
> of time: perhaps it was originally that ctf-subr.c contained nothing but
> wrappers.  I am quite amenable to combining the two, and also to
> splitting the errno-setting stuff out into ctf-error.c with the error
> fetchers.

Meh - whatever you want.  The distribution of the code within the library
is really just up to you.


> +#include <gelf.h>

This header is from the elfutils project right ?  Given that, and the fact
that you are using the types from this header, why are you submitting this
code to the binutils project ?  (Or maybe you are submitting it to both
projects - I have not checked).

In particular the BFD library has its own ELF reading and writing functions 
and its own headers defining the layout of ELF structures.  Unfortunately 
these headers do tend to conflict with the headers from the elfutils project, 
whoch makes combining them problematical.


> +/* Simple doubly-linked list append routine.  This implementation assumes that
> +   each list element contains an embedded ctf_list_t as the first member.
> +   An additional ctf_list_t is used to store the head (l_next) and tail
> +   (l_prev) pointers.  The current head and tail list elements have their
> +   previous and next pointers set to NULL, respectively.  */

You knows this kind of code seems awfully familiar.  I am sure that I have seen
it implemented in lots of different places... :-)


> +void
> +ctf_list_prepend (ctf_list_t * lp, void *new)

I think that using "new" here might be a problem if you try to compile this
source file with a C++ compiler.


> +const char *
> +ctf_strraw (ctf_file_t *fp, uint32_t name)
> +{
> +  ctf_strs_t *ctsp = &fp->ctf_str[CTF_NAME_STID (name)];

My inner paranoia is screaming at code like this.  Unless you
are certain that these functions cannot be called with out of 
range parameters then I would strongly urge checking them before
using them.

Cheers
  Nick