[PATCH] Allow sysdb_search_entry request to return more than one result

[Sumit Bose]

Hi,

this patch makes the sysdb_search_entry request more flexible by
enableing it to return more than one result. I have modified the current
callers so that they only take the first result and send a DEBUG message
if there are more than one results.

bye,
Sumit

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/28/2009 02:55 PM, Sumit Bose wrote:
>  Hi,
>  
>  this patch makes the sysdb_search_entry request more flexible by
>  enableing it to return more than one result. I have modified the current
>  callers so that they only take the first result and send a DEBUG message
>  if there are more than one results.
>  
>  bye,
>  Sumit
>  
>  
>  
>  _______________________________________________
>  sssd-devel mailing list
>  sssd-devel(a)lists.fedorahosted.org
>  https://fedorahosted.org/mailman/listinfo/sssd-devel 
Nack.

In sysdb_search_entry_done() please move state->msgs_count++; above the
talloc_realloc and use state->msgs_count + 1 for the size instead of +2.
It reads better (one extra for the NULL). I'd also prefer if you used
'if (state->msgs_count == 0)' instead of 'if (!state->msgs)' in
LDB_REPLY_DONE, because it's technically possible for state->msgs[0] ==
NULL and this test will still succeed (and be wrong).

In the assorted _recv() functions, you implicitly return the first entry
found. This is a distinct break from the previous functionality, where
sysdb_search_entry_done() would have thrown an error if the count was >
1. I think we need to continue reporting an error here instead of
returning possibly incorrect data.* Besides, if we've gotten a
multiple-value return for a base search, there's something seriously
wrong and this needs to be reported.

* The data could be incorrect because we don't necessarily know whether
the LDB will always return the same user first.

- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkrplWYACgkQeiVVYja6o6MMcQCfVKSdyReoJ2tNFlSdLesFroqc
> npEAn0iMItMh9/J9O/RjsB0JSPzcjS6S
> =ekUQ
> -----END PGP SIGNATURE-----

[Sumit Bose]

On Thu, Oct 29, 2009 at 09:15:23AM -0400, Stephen Gallagher wrote:
>  -----BEGIN PGP SIGNED MESSAGE-----
>  Hash: SHA1
>  
>  On 10/28/2009 02:55 PM, Sumit Bose wrote:
>  > Hi,
>  > 
>  > this patch makes the sysdb_search_entry request more flexible by
>  > enableing it to return more than one result. I have modified the current
>  > callers so that they only take the first result and send a DEBUG message
>  > if there are more than one results.
>  > 
>  > bye,
>  > Sumit
>  > 
>  > 
>  > 
>  > _______________________________________________
>  > sssd-devel mailing list
>  > sssd-devel(a)lists.fedorahosted.org
>  > https://fedorahosted.org/mailman/listinfo/sssd-devel
>  
>  Nack.
>  
>  In sysdb_search_entry_done() please move state->msgs_count++; above the
>  talloc_realloc and use state->msgs_count + 1 for the size instead of +2.
>  It reads better (one extra for the NULL). I'd also prefer if you used
>  'if (state->msgs_count == 0)' instead of 'if (!state->msgs)' in
>  LDB_REPLY_DONE, because it's technically possible for state->msgs[0] ==
>  NULL and this test will still succeed (and be wrong).
>  
>  In the assorted _recv() functions, you implicitly return the first entry
>  found. This is a distinct break from the previous functionality, where
>  sysdb_search_entry_done() would have thrown an error if the count was >
>  1. I think we need to continue reporting an error here instead of
>  returning possibly incorrect data.* Besides, if we've gotten a
>  multiple-value return for a base search, there's something seriously
>  wrong and this needs to be reported.
>  
>  * The data could be incorrect because we don't necessarily know whether
>  the LDB will always return the same user first.
>  
>  - -- 
>  Stephen Gallagher
>  RHCE 804006346421761
>  
>  Delivering value year after year.
>  Red Hat ranks #1 in value among software vendors.
>  http://www.redhat.com/promo/vendor/
> >  -----BEGIN PGP SIGNATURE-----
> >  Version: GnuPG v1.4.9 (GNU/Linux)
> >  Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
> >  
> >  iEYEARECAAYFAkrplWYACgkQeiVVYja6o6MMcQCfVKSdyReoJ2tNFlSdLesFroqc
> >  npEAn0iMItMh9/J9O/RjsB0JSPzcjS6S
> >  =ekUQ
> >  -----END PGP SIGNATURE----- 
>  
ok, the _recv functions return EFAULT now and
the
'if (state->msgs_count == 0)' check is used. I haven't changed the
realloc section because it would result in a 'state->msgs_count -1'
in an index.

Thanks for the review.

bye,
Sumit

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/29/2009 10:02 AM, Sumit Bose wrote:
>  On Thu, Oct 29, 2009 at 09:15:23AM -0400, Stephen Gallagher wrote:
>  On 10/28/2009 02:55 PM, Sumit Bose wrote:
> >>> Hi,
> >>>
> >>> this patch makes the sysdb_search_entry request more flexible by
> >>> enableing it to return more than one result. I have modified the current
> >>> callers so that they only take the first result and send a DEBUG message
> >>> if there are more than one results.
> >>>
> >>> bye,
> >>> Sumit
> >>>
> >>>
> >>>
> >>> _______________________________________________
> >>> sssd-devel mailing list
> >>> sssd-devel(a)lists.fedorahosted.org
> >>> https://fedorahosted.org/mailman/listinfo/sssd-devel
>  
>  Nack.
>  
>  In sysdb_search_entry_done() please move state->msgs_count++; above the
>  talloc_realloc and use state->msgs_count + 1 for the size instead of +2.
>  It reads better (one extra for the NULL). I'd also prefer if you used
>  'if (state->msgs_count == 0)' instead of 'if (!state->msgs)' in
>  LDB_REPLY_DONE, because it's technically possible for state->msgs[0] ==
>  NULL and this test will still succeed (and be wrong).
>  
>  In the assorted _recv() functions, you implicitly return the first entry
>  found. This is a distinct break from the previous functionality, where
>  sysdb_search_entry_done() would have thrown an error if the count was >
>  1. I think we need to continue reporting an error here instead of
>  returning possibly incorrect data.* Besides, if we've gotten a
>  multiple-value return for a base search, there's something seriously
>  wrong and this needs to be reported.
>  
>  * The data could be incorrect because we don't necessarily know whether
>  the LDB will always return the same user first.
>   
Ack

>  ok, the _recv functions return EFAULT now and the
>  'if (state->msgs_count == 0)' check is used. I haven't changed the
>  realloc section because it would result in a 'state->msgs_count -1'
>  in an index. 
>  Thanks for the review. 
>  bye,
>  Sumit 


>  _______________________________________________
>  sssd-devel mailing list
>  sssd-devel(a)lists.fedorahosted.org
>  https://fedorahosted.org/mailman/listinfo/sssd-devel 

- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkrpq7YACgkQeiVVYja6o6NT6wCfTN+yoN5Xgq2LOAprpclXjsKB
> ATUAoJPs4PYBFiuMdf7ibCiOj0yAIhmq
> =wxmJ
> -----END PGP SIGNATURE-----

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/29/2009 10:50 AM, Stephen Gallagher wrote:
>  On 10/29/2009 10:02 AM, Sumit Bose wrote:
> > On Thu, Oct 29, 2009 at 09:15:23AM -0400, Stephen Gallagher wrote:
> > On 10/28/2009 02:55 PM, Sumit Bose wrote:
> >>>> Hi,
> >>>>
> >>>> this patch makes the sysdb_search_entry request more flexible by
> >>>> enableing it to return more than one result. I have modified the current
> >>>> callers so that they only take the first result and send a DEBUG message
> >>>> if there are more than one results.
> >>>>
> >>>> bye,
> >>>> Sumit
> >>>>
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> sssd-devel mailing list
> >>>> sssd-devel(a)lists.fedorahosted.org
> >>>> https://fedorahosted.org/mailman/listinfo/sssd-devel
>  
> > Nack.
>  
> > In sysdb_search_entry_done() please move state->msgs_count++; above the
> > talloc_realloc and use state->msgs_count + 1 for the size instead of +2.
> > It reads better (one extra for the NULL). I'd also prefer if you used
> > 'if (state->msgs_count == 0)' instead of 'if (!state->msgs)'
> in
> > LDB_REPLY_DONE, because it's technically possible for state->msgs[0] ==
> > NULL and this test will still succeed (and be wrong).
>  
> > In the assorted _recv() functions, you implicitly return the first entry
> > found. This is a distinct break from the previous functionality, where
> > sysdb_search_entry_done() would have thrown an error if the count was >
> > 1. I think we need to continue reporting an error here instead of
> > returning possibly incorrect data.* Besides, if we've gotten a
> > multiple-value return for a base search, there's something seriously
> > wrong and this needs to be reported.
>  
> > * The data could be incorrect because we don't necessarily know whether
> > the LDB will always return the same user first.
>  
>  
>  Ack
>  
> > ok, the _recv functions return EFAULT now and the
> > 'if (state->msgs_count == 0)' check is used. I haven't changed the
> > realloc section because it would result in a 'state->msgs_count -1'
> > in an index.
>  
> > Thanks for the review.
>  
> > bye,
> > Sumit
>  
>  
>  
> > _______________________________________________
> > sssd-devel mailing list
> > sssd-devel(a)lists.fedorahosted.org
> > https://fedorahosted.org/mailman/listinfo/sssd-devel
>  
>   
Pushed to master.
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkrprGUACgkQeiVVYja6o6PwFQCglTMJ02pqdOx/Z9D70zYfF7k6
> PLsAnRbsXMeJIJ6u90/n1I+z/agLstoR
> =FkYi
> -----END PGP SIGNATURE-----