[PATCHES] Use pattern #elif defined(identifier)

[Lukas Slebodnik]

ehlo,

We had in source code following pattern #elif HAVE_<name>
It worked because undefined identifier(in some cases) was evaluated to 0.
But we do not care about value of HAVE_SOMETHING. We just need to know
whether identifier was defined.

There is not equivalent to #ifdef (short for of #if definded)
We need to use long form: #elif defined HAVE_<name>

It causes also compiler warning with enabled compiler flag -Wundef.

LS

[Pavel Reichl]

On Mon, 2014-03-10 at 09:20 +0100, Lukas Slebodnik wrote:
> ehlo,
>
> We had in source code following pattern #elif HAVE_<name>
> It worked because undefined identifier(in some cases) was evaluated to 0.
> But we do not care about value of HAVE_SOMETHING. We just need to know
> whether identifier was defined.
>
> There is not equivalent to #ifdef (short for of #if definded)
> We need to use long form: #elif defined HAVE_<name>
>
> It causes also compiler warning with enabled compiler flag -Wundef.
>
> LS
> _______________________________________________
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Hello Lukas,

Patch 1:
While testing I applied 3rd patch first and it (as expected) raised some
errors which were fixed by 2nd patch. 
I can't see any problems which are addressed by this patch. There aren't
any #if directives or any other usage of (#define) constants from
config.h.  Could you possibly describe in more detail the benefit of
this patch, please?

Patch 2:
ACK

Patch 3:
ACK - I think it's really useful - like it:-).

Thanks.

Pavel Reichl

[Lukas Slebodnik]

On (12/03/14 17:52), Pavel Reichl wrote:
> On Mon, 2014-03-10 at 09:20 +0100, Lukas Slebodnik wrote:
> > ehlo,
> >
> > We had in source code following pattern #elif HAVE_<name>
> > It worked because undefined identifier(in some cases) was evaluated to 0.
> > But we do not care about value of HAVE_SOMETHING. We just need to know
> > whether identifier was defined.
> >
> > There is not equivalent to #ifdef (short for of #if definded)
> > We need to use long form: #elif defined HAVE_<name>
> >
> > It causes also compiler warning with enabled compiler flag -Wundef.
> >
> > LS
> > _______________________________________________
> > sssd-devel mailing list
> > [e-mail]
> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
> Hello Lukas,
>
> Patch 1:
> While testing I applied 3rd patch first and it (as expected) raised some
> errors which were fixed by 2nd patch.
> I can't see any problems which are addressed by this patch. There aren't
> any #if directives or any other usage of (#define) constants from
> config.h.  Could you possibly describe in more detail the benefit of
> this patch, please?

In header file sss_utf8.h there are lines:

#ifdef HAVE_LIBUNISTRING
#include <unistr.h>
#include <unicase.h>
#elif defined(HAVE_GLIB2)
#include <glib.h>
#endif

But header file config.h was not included before header file sss_utf8.h in some
places. So neither unistring header files nor glib header files were included.

I realised these header files are necessary only in implementation module
sss_utf8.c. I move part with ifdef into sss_utf8.c

New patch is attached.
> Patch 2:
> ACK
>
> Patch 3:
> ACK - I think it's really useful - like it:-).
>
> Thanks.
>
> Pavel Reichl

[Pavel Reichl]

On Fri, 2014-03-14 at 12:58 +0100, Lukas Slebodnik wrote:
> On (12/03/14 17:52), Pavel Reichl wrote:
> > On Mon, 2014-03-10 at 09:20 +0100, Lukas Slebodnik wrote:
> > > ehlo,
> > >
> > > We had in source code following pattern #elif HAVE_<name>
> > > It worked because undefined identifier(in some cases) was evaluated to 0.
> > > But we do not care about value of HAVE_SOMETHING. We just need to know
> > > whether identifier was defined.
> > >
> > > There is not equivalent to #ifdef (short for of #if definded)
> > > We need to use long form: #elif defined HAVE_<name>
> > >
> > > It causes also compiler warning with enabled compiler flag -Wundef.
> > >
> > > LS
> > > _______________________________________________
> > > sssd-devel mailing list
> > > [e-mail]
> > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> >
> > Hello Lukas,
> >
> > Patch 1:
> > While testing I applied 3rd patch first and it (as expected) raised some
> > errors which were fixed by 2nd patch.
> > I can't see any problems which are addressed by this patch. There aren't
> > any #if directives or any other usage of (#define) constants from
> > config.h.  Could you possibly describe in more detail the benefit of
> > this patch, please?
>
> In header file sss_utf8.h there are lines:
>
> #ifdef HAVE_LIBUNISTRING
> #include <unistr.h>
> #include <unicase.h>
> #elif defined(HAVE_GLIB2)
> #include <glib.h>
> #endif
>
> But header file config.h was not included before header file sss_utf8.h in some
> places. So neither unistring header files nor glib header files were included.
>
> I realised these header files are necessary only in implementation module
> sss_utf8.c. I move part with ifdef into sss_utf8.c
>
> New patch is attached.
> > Patch 2:
> > ACK
> >
> > Patch 3:
> > ACK - I think it's really useful - like it:-).
> >
> > Thanks.
> >
> > Pavel Reichl
>
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Thank you Lukas,

new patches apply and I can't see any warnings.

ACK to all patches.

Pavel Reichl

[Jakub Hrozek]

On Fri, Mar 14, 2014 at 01:52:55PM +0100, Pavel Reichl wrote:
> On Fri, 2014-03-14 at 12:58 +0100, Lukas Slebodnik wrote:
> > On (12/03/14 17:52), Pavel Reichl wrote:
> > > On Mon, 2014-03-10 at 09:20 +0100, Lukas Slebodnik wrote:
> > > > ehlo,
> > > >
> > > > We had in source code following pattern #elif HAVE_<name>
> > > > It worked because undefined identifier(in some cases) was evaluated to 0.
> > > > But we do not care about value of HAVE_SOMETHING. We just need to know
> > > > whether identifier was defined.
> > > >
> > > > There is not equivalent to #ifdef (short for of #if definded)
> > > > We need to use long form: #elif defined HAVE_<name>
> > > >
> > > > It causes also compiler warning with enabled compiler flag -Wundef.
> > > >
> > > > LS
> > > > _______________________________________________
> > > > sssd-devel mailing list
> > > > [e-mail]
> > > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> > >
> > > Hello Lukas,
> > >
> > > Patch 1:
> > > While testing I applied 3rd patch first and it (as expected) raised some
> > > errors which were fixed by 2nd patch.
> > > I can't see any problems which are addressed by this patch. There aren't
> > > any #if directives or any other usage of (#define) constants from
> > > config.h.  Could you possibly describe in more detail the benefit of
> > > this patch, please?
> >
> > In header file sss_utf8.h there are lines:
> >
> > #ifdef HAVE_LIBUNISTRING
> > #include <unistr.h>
> > #include <unicase.h>
> > #elif defined(HAVE_GLIB2)
> > #include <glib.h>
> > #endif
> >
> > But header file config.h was not included before header file sss_utf8.h in some
> > places. So neither unistring header files nor glib header files were included.
> >
> > I realised these header files are necessary only in implementation module
> > sss_utf8.c. I move part with ifdef into sss_utf8.c
> >
> > New patch is attached.
> > > Patch 2:
> > > ACK
> > >
> > > Patch 3:
> > > ACK - I think it's really useful - like it:-).
> > >
> > > Thanks.
> > >
> > > Pavel Reichl
> >
> > sssd-devel mailing list
> > [e-mail]
> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
> Thank you Lukas,
>
> new patches apply and I can't see any warnings.
>
> ACK to all patches.
>
> Pavel Reichl

Pushed all to master.