[Libguestfs] [PATCH 1/4] Add a core_pattern debug command

[Richard W.M. Jones]

On Thu, Aug 26, 2010 at 12:11:58PM +0100, Matthew Booth wrote:
>  static struct cmd cmds[] = {
>    { "help", debug_help },
> @@ -61,6 +63,7 @@ static struct cmd cmds[] = {
>    { "ll", debug_ll },
>    { "segv", debug_segv },
>    { "sh", debug_sh },
> +  { "core_pattern", debug_core_pattern },

These are in alphabetical order (apart from "help") so we should keep
them in that order.

> +static char *
> +debug_core_pattern (const char *subcmd, int argc, char *const *const argv)
> +{
> +  if (argc < 1) {
> +    reply_with_error ("core_pattern: expecting a core pattern");
> +  }

Missing 'return NULL;'.

> +  const char *pattern = argv[0];
> +  const size_t pattern_len = strlen(pattern);
> +
> +#define CORE_PATTERN "/proc/sys/kernel/core_pattern"
> +  int fd = open (CORE_PATTERN, O_WRONLY);
> +  if (fd == -1) {
> +    reply_with_error ("open: " CORE_PATTERN);

These should (all 4 of them) use reply_with_perror, since I assume you
want to communicate the reason for the failure,

> +  return strdup("ok");

Interesting ..  hadn't thought about what we might return for commands
which succeed without returning anything, but I guess "ok" is as good
as anything.  It's only a debug command after all.  However I'd
prefer:

  char *ret = strdup ("ok");
  if (ret == NULL) {
    reply_with_perror ("strdup");
    return NULL;
  }
  return ret;

In general this patch is (still) good and we should integrate this
functionality as soon as possible.  Post an updated patch which fixes
all the above and we should be able to get it in straightaway.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v