Re: [PATCH 2/7] Add platform agnostic dynamic loading code.

Tom Tromey
2011-08-10T21:01:35Z

>>>>> "Sanjoy" == Sanjoy Das <sanjoy@playingwithpointers.com> writes:

Sanjoy> * gdb-dlfcn.h, gdb-dlfcn.c: New.
Sanjoy> * Makefile.in: Add gdb_dlcfn.c and gdb_dlcfn.h to the build system.
Sanjoy> * configure.ac, config.in: Check for -ldl and accordingly define HAVE_LIBDL.

Looks pretty good.

Sanjoy> diff --git a/gdb/gdb-dlfcn.c b/gdb/gdb-dlfcn.c
Sanjoy> new file mode 100644
Sanjoy> index 0000000..91e8a22
Sanjoy> --- /dev/null
Sanjoy> +++ b/gdb/gdb-dlfcn.c
Sanjoy> @@ -0,0 +1,49 @@
Sanjoy> +#include "gdb-dlfcn.h"
Sanjoy> +
Sanjoy> +#include "config.h"

This file needs a copyright header.
Instead of config.h, include defs.h first thing.

Sanjoy> +int
Sanjoy> +gdb_dlclose (void *handle)

Needs a comment describing the return value.

For a new self-contained module like this, I'd rather the comments be in
the header than in the implementation.

Tom