[SSSD] [PATCH] ELAPI Compatibility code for getifaddr()
Dmitri Pal
dpal at redhat.com
Wed Oct 14 18:27:09 UTC 2009
After some thinking I just fixed the configure.ac and added a new header
file.
The updated patch attached.
Dmitri Pal wrote:
> Jakub Hrozek wrote:
>
>> On 10/07/2009 03:51 PM, Dmitri Pal wrote:
>>
>>> Ticket: https://fedorahosted.org/sssd/ticket/94
>>> See patch comment.
>>>
>> Ack to the actual C code (I think the #defines to types are justified in
>> this case), Nack to the WITH_COMPAT macro.
>>
>> You always define HAVE_GETIFADDRS, even though it is defined to 0,
>> #ifdef will pass. Also the logic should be different, I think -- the
>> default (same as --disable-compat) should be to check for getifaddrs()
>> and error out during configure if not found, if found, set
>> HAVE_GETIFADDRS. If --enable-compat is given, don't bother looking up
>> anything.
>>
>> This seems to work for me:
>> ---
>> AC_DEFUN([WITH_COMPAT],
>> [
>> AC_ARG_ENABLE([compat],
>> [AC_HELP_STRING([--enable-compat],
>> [use platform compatibility code])
>> ],
>> [
>> use_compat=$enableval
>> ],
>> [
>> use_compat="no"
>> ]
>> )
>>
>> if test x"$use_compat" == x"no"; then
>> AC_CHECK_FUNC([getifaddrs],
>> [
>> AC_DEFINE([HAVE_GETIFADDRS], [1],
>> [Define if platform has getifaddrs()])
>> ],
>> [ AC_MSG_ERROR([getifaddrs() not found]) ]
>> )
>> fi
>>
>> ])
>> ---
>>
>>
>
>
> The logic I was implementing is :
> If there is no getifaddr use compat mode - no setting needed for this case
> If getifaddr is there use getifaddr unless you explicitly overwrite it
> for testing purposes with the compat mode.
> I think the setting should be optional and used only on very rare occasions.
> It should not be with value. Value does not add anything other than need
> to validate it.
> By default it should just build the right thing for the platform.
>
> This is the code:
>
> AC_DEFUN([WITH_COMPAT],
> [ AC_ARG_ENABLE([compat],
> [AC_HELP_STRING([--enable-compat],
> [use platform compatibility code])
> ],
> [AC_DEFINE([HAVE_GETIFADDRS], [0],
> [Force ELAPI to use compatibility code for
> getifaddrs()]
> )
> ],
> [AC_CHECK_FUNC([getifaddrs],
> AC_DEFINE([HAVE_GETIFADDRS], [1],
> [Define if platform has
> getifaddrs()])
> )
> ]
> )
>
> ])
>
> I think that if I just remove the first AC_DEFINE the HAVE_GETIFADDRS
> will be defined only
> if we are not forcing compat mode and have getifaddr.
> I think this is cleaner and simpler.
> Like this:
>
> AC_DEFUN([WITH_COMPAT],
> [ AC_ARG_ENABLE([compat],
> [AC_HELP_STRING([--enable-compat],
> [use platform compatibility code])
> ],
> ,
> [AC_CHECK_FUNC([getifaddrs],
> AC_DEFINE([HAVE_GETIFADDRS], [1],
> [Define if platform has
> getifaddrs()])
> )
> ]
> )
>
> ])
>
>
> Ok?
>
>
>
>> Also, a little unrelated question -- I noticed that EOK is defined twice
>> in ELAPI code, is there a reason for that? Would it make more sense to
>> define this common stuff in some header file shared by all ELAPI modules?
>>
>>
>
> Well, yes and no.
> Having a common header for just one define that is included into all
> modules seems a bit of an overhead to me.
> I suggest you open a ticket and we address this when/if we have time.
> I do not like the defines in multiple places either but it is a balance
> between good enough and perfect :-).
>
>
>> Jakub
>>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel
>
>
--
Thank you,
Dmitri Pal
Engineering Manager IPA project,
Red Hat Inc.
-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ELAPI-Compatibility-code-for-getifaddr.patch
Type: text/x-patch
Size: 19457 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20091014/ec12131d/attachment.bin>
More information about the sssd-devel
mailing list