[Libvir] PATCH: 5/10: public auth callback API

[Jim Meyering]

"Daniel P. Berrange" <berrange at redhat.com> wrote:
> This patch introduces the public & driver APIs for collecting auth
> credentials via a callback. This allows username/password based auth
> schemes to be used.
...
> diff -r ec2d8c632fd9 Makefile.am
> --- a/Makefile.am	Wed Nov 28 15:00:45 2007 -0500
> +++ b/Makefile.am	Wed Nov 28 20:35:23 2007 -0500
> @@ -1,6 +1,6 @@
>  ## Process this file with automake to produce Makefile.in
>
> -SUBDIRS = src qemud proxy include docs @PYTHON_SUBDIR@ tests po m4 scripts
> +SUBDIRS = include src qemud proxy docs @PYTHON_SUBDIR@ tests po m4 scripts

I guess you did the above because something in src/ or qemud/ now
requires the generated libvirt.h?

...
> diff -r ec2d8c632fd9 qemud/remote.c
> --- a/qemud/remote.c	Wed Nov 28 15:00:45 2007 -0500
> +++ b/qemud/remote.c	Wed Nov 28 20:35:24 2007 -0500
> @@ -2062,7 +2062,7 @@ remoteDispatchAuthList (struct qemud_ser
>                          remote_auth_list_ret *ret)
>  {
>      ret->types.types_len = 1;
> -    if ((ret->types.types_val = calloc (ret->types.types_len, sizeof (remote_auth_type))) == NULL) {
> +    if ((ret->types.types_val = calloc (ret->types.types_len, sizeof (u_int))) == NULL) {

This is a good demonstration of why using sizeof (type name) is fragile.

Given code like T *var = calloc (n, sizeof (T));
I prefer this:  T *var = calloc (n, sizeof (*var));

Then when type T changes, like it just did above, you don't
have to be careful to update all sizeof arguments that are somehow
related to "var".

Not to say you should change your patch.
If no one objects on principle, I'll do the global cleanup, eventually.

BTW, changing the above code gives this:

  ret->types.types_val = calloc (ret->types.types_len,
                                 sizeof (*(ret->types.types_val))) == NULL) {

which is a good argument for using a helper macro.
e.g.,

#define CALLOC(Var,N) do { (Var) = calloc ((N), sizeof (*(Var))); } while (0)

Of course, if you can have statement-expression macros (gcc-specific),
you can do better, but I don't think we can depend on compiling with gcc.