[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