[SSSD] [PATCH] ELAPI Compatibility code for getifaddr()

Dmitri Pal dpal at redhat.com
Wed Oct 14 17:57:49 UTC 2009


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/




More information about the sssd-devel mailing list