[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH] Recognise cd-rom devices in devsparts.c



On Mon, Aug 03, 2009 at 04:25:55PM +0100, Matthew Booth wrote:
> +  /* Add a device to the list of devices */
> +  int add_to_device_list(const char *device) {
> +      char dev_path[256];
> +      snprintf (dev_path, sizeof dev_path, "/dev/%s", device);

So we really are going for nested functions then.

There is something about nested functions which is bad (in the context
of GCC).  I don't recall exactly what it is now.  It's something like
it uses stack trampolines, or breaks something ...

Anyway, in this case:

(1) Indenting is wrong.

(2) The nested function should return 0 / -1 (instead of true/false)
to be consistent with everything else.

Otherwise it looks good.

Please also ensure it doesn't break a complete run of 'make check',
since list-devices/list-partitions are *very* widely used functions,
probably the most important functions after 'add-device' and 'launch'.

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 75 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]