[PATCH] DHASH: Don't use backward jumps

[Ondrej Kos]

https://fedorahosted.org/sssd/ticket/1855

I couldn't let it this way when I saw the code :)
Patch is attached

Ondra

[Lukas Slebodnik]

On (02/04/13 17:44), Ondrej Kos wrote:
> https://fedorahosted.org/sssd/ticket/1855
>
> I couldn't let it this way when I saw the code :)
> Patch is attached
> Ondra
>
> Ondrej Kos
> Associate Software Engineer
> Identity Management
> Red Hat Czech
>
> phone: [phone]
> cell:  [phone]
> ext:   82-62558
> loc:   1013 Brno 1 office
> irc:   okos @ #sssd #brno
> diff --git a/dhash/dhash.h b/dhash/dhash.h
> index baa0d6aa88b767b63018127c4ac454922fdd77d8..7f44c57f8091c70766c9d7fccb166e81b377721e 100644
> --- a/dhash/dhash.h
> +++ b/dhash/dhash.h
> @@ -133,6 +133,14 @@ typedef enum
>     HASH_ENTRY_DESTROY
> } hash_destroy_enum;
>
> +typedef enum {
> HI_STATE_0,
> HI_STATE_1,
> HI_STATE_2,
> HI_STATE_3A,
> HI_STATE_3B
>
> +} hash_iter_state;

This enum is only used in static function hash_iter_next().
It is unnecessarily to be available in public API.
I think that this typedef could be safety moved to implementation file dhash.c
> typedef struct hash_key_t {
>     hash_key_enum type;
>     union {
> -- 
> 1.8.1.4
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

[Ondrej Kos]

On 04/05/2013 01:52 PM, Lukas Slebodnik wrote:
> On (02/04/13 17:44), Ondrej Kos wrote:
> > https://fedorahosted.org/sssd/ticket/1855
> >
> > I couldn't let it this way when I saw the code :)
> > Patch is attached
> > Ondra
> >
> > Ondrej Kos
> > Associate Software Engineer
> > Identity Management
> > Red Hat Czech
> >
> > phone: [phone]
> > cell:  [phone]
> > ext:   82-62558
> > loc:   1013 Brno 1 office
> > irc:   okos @ #sssd #brno
> > diff --git a/dhash/dhash.h b/dhash/dhash.h
> > index baa0d6aa88b767b63018127c4ac454922fdd77d8..7f44c57f8091c70766c9d7fccb166e81b377721e 100644
> > --- a/dhash/dhash.h
> > +++ b/dhash/dhash.h
> > @@ -133,6 +133,14 @@ typedef enum
> >      HASH_ENTRY_DESTROY
> > } hash_destroy_enum;
> >
> > +typedef enum {
> > HI_STATE_0,
> > HI_STATE_1,
> > HI_STATE_2,
> > HI_STATE_3A,
> > HI_STATE_3B
> >
> > +} hash_iter_state;
>
> This enum is only used in static function hash_iter_next().
> It is unnecessarily to be available in public API.
> I think that this typedef could be safety moved to implementation file dhash.c
> > typedef struct hash_key_t {
> >      hash_key_enum type;
> >      union {
> > --
> > 1.8.1.4

Thanks for review Lukas,

New patch attached.

Ondra

[Lukas Slebodnik]

On (05/04/13 14:02), Ondrej Kos wrote:
> On 04/05/2013 01:52 PM, Lukas Slebodnik wrote:
> > On (02/04/13 17:44), Ondrej Kos wrote:
> > > https://fedorahosted.org/sssd/ticket/1855
> > >
> > > I couldn't let it this way when I saw the code :)
> > > Patch is attached
> > > Ondra
> > >
> > > Ondrej Kos
> > > Associate Software Engineer
> > > Identity Management
> > > Red Hat Czech
> > >
> > > phone: [phone]
> > > cell:  [phone]
> > > ext:   82-62558
> > > loc:   1013 Brno 1 office
> > > irc:   okos @ #sssd #brno
> > > diff --git a/dhash/dhash.h b/dhash/dhash.h
> > > index baa0d6aa88b767b63018127c4ac454922fdd77d8..7f44c57f8091c70766c9d7fccb166e81b377721e 100644
> > > --- a/dhash/dhash.h
> > > +++ b/dhash/dhash.h
> > > @@ -133,6 +133,14 @@ typedef enum
> > >     HASH_ENTRY_DESTROY
> > > } hash_destroy_enum;
> > >
> > > +typedef enum {
> > > HI_STATE_0,
> > > HI_STATE_1,
> > > HI_STATE_2,
> > > HI_STATE_3A,
> > > HI_STATE_3B
> > >
> > > +} hash_iter_state;
> >
> > This enum is only used in static function hash_iter_next().
> > It is unnecessarily to be available in public API.
> > I think that this typedef could be safety moved to implementation file dhash.c
> > > typedef struct hash_key_t {
> > >     hash_key_enum type;
> > >     union {
> > > --
> > > 1.8.1.4
>
> Thanks for review Lukas,
>
> New patch attached.
>
> Ondra
>
> -- 
> Ondrej Kos
> Associate Software Engineer
> Identity Management
> Red Hat Czech

Ack

LS

[Ondrej Kos]

On 04/05/2013 02:10 PM, Lukas Slebodnik wrote:
> On (05/04/13 14:02), Ondrej Kos wrote:
> > On 04/05/2013 01:52 PM, Lukas Slebodnik wrote:
> > > On (02/04/13 17:44), Ondrej Kos wrote:
> > > > https://fedorahosted.org/sssd/ticket/1855
> > > >
> > > > I couldn't let it this way when I saw the code :)
> > > > Patch is attached
> > > > Ondra
> > > >
> > > > Ondrej Kos
> > > > Associate Software Engineer
> > > > Identity Management
> > > > Red Hat Czech
> > > >
> > > > phone: [phone]
> > > > cell:  [phone]
> > > > ext:   82-62558
> > > > loc:   1013 Brno 1 office
> > > > irc:   okos @ #sssd #brno
> > > > diff --git a/dhash/dhash.h b/dhash/dhash.h
> > > > index baa0d6aa88b767b63018127c4ac454922fdd77d8..7f44c57f8091c70766c9d7fccb166e81b377721e 100644
> > > > --- a/dhash/dhash.h
> > > > +++ b/dhash/dhash.h
> > > > @@ -133,6 +133,14 @@ typedef enum
> > > >      HASH_ENTRY_DESTROY
> > > > } hash_destroy_enum;
> > > >
> > > > +typedef enum {
> > > > HI_STATE_0,
> > > > HI_STATE_1,
> > > > HI_STATE_2,
> > > > HI_STATE_3A,
> > > > HI_STATE_3B
> > > >
> > > > +} hash_iter_state;
> > >
> > > This enum is only used in static function hash_iter_next().
> > > It is unnecessarily to be available in public API.
> > > I think that this typedef could be safety moved to implementation file dhash.c
> > > > typedef struct hash_key_t {
> > > >      hash_key_enum type;
> > > >      union {
> > > > --
> > > > 1.8.1.4
> >
> > Thanks for review Lukas,
> >
> > New patch attached.
> >
> > Ondra
> >
> > --
> > Ondrej Kos
> > Associate Software Engineer
> > Identity Management
> > Red Hat Czech
>
> Ack
>
> LS

Pushed to master.