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

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



On Mon, Dec 03, 2007 at 07:33:49PM +0100, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange 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?

Yes, everything in src & qemud dirs includes  libvirt.h, and if you ever
change libvirt.h.in, then we need to make sure libvirt.h is re-generated
before compiling src & qemud otherwise we potentially mis-compile. Its
a minor thing, but worth getting right to avoid suprising developers like
myself in future :-)

> ...
> > 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.

Sounds reasonable to me.

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 


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