On Wed, 2012-08-08 at 08:56 +0200, Ondrej Kos wrote:
> Strange, I've created the patch with this alias, so patience flag should 
> be on
> alias gfp='git format-patch -M -C --patience --full-index'
> 
> As for the while -> do-while, if I'd do that, then it would need another 
> condition
> if (status == NSS_STATUS_TRYAGAIN)
> for the block of code which adjusts buffer size on lines 1250-1263. It 
> seemed a bit pointless when you can use just one condition in while. So, 
> should i leave it this way, or really use do-while with additional if?

Well in general I prefer to have assignment of a variable as return of a
function and testing it's value as separate lines as that makes it much
easier to read but also makes it easier to step through in a debugger.

So as long as setting and testing status are separate I am fine with any
solution really.

Simo.

> Ondrej
> 
> On 08/07/2012 03:58 PM, Simo Sorce wrote:
> > One comment and one nitpick.
> >
> > Comment: polease use --patience flag to git format-patch so that the
> > patches are more readable.
> >
> > Nitpick:
> >
> > On Tue, 2012-08-07 at 15:26 +0200, Ondrej Kos wrote:
> >> -                                     &num, &gids, limit, &ret);
> >> -    switch (status) {
> >> -    case NSS_STATUS_TRYAGAIN:
> >> +    while ( (status = ctx->ops.initgroups_dyn(pwd->pw_name,
> >> pwd->pw_gid,
> >> +                                    &num_gids, &num, &gids, limit,
> >> &ret))
> >> +                                    == NSS_STATUS_TRYAGAIN) {
> >>           /* buffer too small ? */
> >
> > here please use a do/whiel loop to make code more readable:
> >
> > do {
> >     status = ctx->ops.initgrou ...
> >
> > } while (status == NSS_STATUS_TRYAGAIN);
> >
> >
> > Simo.
> >
> >
> 
> 


-- 
Simo Sorce * Red Hat, Inc * New York

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to