[libvirt] [PATCH v5 6/9] nss: Implement _nss_libvirt_gethostbyname3_r

[Martin Kletzander]

On Fri, Mar 18, 2016 at 04:18:15PM +0100, Michal Privoznik wrote:
>On 18.03.2016 15:10, Martin Kletzander wrote:
>> On Tue, Mar 15, 2016 at 06:05:53PM +0100, Michal Privoznik wrote:
>>> The implementation is pretty straightforward. Moreover, because
>>> of the nature of things, gethostbyname_r and gethostbyname2_r can
>>> be implemented at the same time too.
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>>> ---
>>> config-post.h              |  24 ++++
>>> src/Makefile.am            |  57 ++++++++
>>> src/util/virfile.c         |   3 +-
>>> src/util/virfile.h         |  10 +-
>>> src/util/virlease.c        |   1 +
>>> tests/Makefile.am          |   2 +-
>>> tools/Makefile.am          |   5 +
>>> tools/nss/libvirt_nss.c    | 336
>>> ++++++++++++++++++++++++++++++++++++++++++++-
>>> tools/nss/libvirt_nss.h    |  14 +-
>>> tools/nss/libvirt_nss.syms |   4 +-
>>> 10 files changed, 447 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/src/util/virfile.h b/src/util/virfile.h
>>> index 312f226..50a3995 100644
>>> --- a/src/util/virfile.h
>>> +++ b/src/util/virfile.h
>>> @@ -30,7 +30,15 @@
>>> # include <dirent.h>
>>>
>>> # include "internal.h"
>>> -# include "virstoragefile.h"
>>> +
>>> +/* Okay, this is not nice, but we want resulting nss module as
>>> + * small as possible. Including virstoragefile.h would drag in
>>> + * libxml2 dependencies which is unfavorable. */
>>> +# ifdef LIBVIRT_NSS
>>> +#  define virStorageFileFormat int
>>> +# else
>>> +#  include "virstoragefile.h"
>>> +# endif
>>>
>>
>> I agree with Daniel here.  Just add LIBXML2_CFLAGS to needed targets so
>> that it compiles properly.  And don't compile in the virstoragefile.c.
>>
>>> typedef enum {
>>>     VIR_FILE_CLOSE_PRESERVE_ERRNO = 1 << 0,
>>> diff --git a/src/util/virlease.c b/src/util/virlease.c
>>> index 910c003..920ebaf 100644
>>> --- a/src/util/virlease.c
>>> +++ b/src/util/virlease.c
>>> @@ -30,6 +30,7 @@
>>> #include "virstring.h"
>>> #include "virerror.h"
>>> #include "viralloc.h"
>>> +#include "virutil.h"
>>>
>>
>> This should be in the patch where you introduce virlease.c, I guess.
>
>It was here because after I started including virstoragefile.h
>conditionally, I experienced couple of build errors which proven to be
>due to a missing include of virutil.h. If we include virstoragefile.h
>without any condition, just like it is now, virutil.h is included
>indirectly from there as well. Yes, we can be nice and include it here
>directly. But truth to be told I'm tired of putting every little change
>into its own commit. I can't put this change into 1/9 because then it
>wouldn't be just a pure code movement. I need to save it for a separate
>patch then. And write a sensible commit message (which will end up being
>longer than change itself). D'oh!
>

I would say just take all of such cleanups and put them together to one
commit, but others will tell you not to do that.  We should come up with
some universal works-for-all way to do this.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160318/356a9398/attachment-0001.sig>