On Fri, Oct 11, 2013 at 12:53:48PM +0200, Benjamin Franzke wrote:
2013/10/11 Sumit Bose <sbose(a)redhat.com>
> On Thu, Oct 10, 2013 at 02:41:54PM +0200, Benjamin Franzke wrote:
> > Thanks for your review!
> >
> <snip>
> >
> > The patch with your concers fixed is attached.
>
> Thank you for the new version. I only have two comments left:
>
> + if (handle == NULL || errmsg == NULL) {
> + *errmsg = strerror(EINVAL);
>
> you cannot return an error message if errmsg == NULL.
>
yes, of course not :)
had to leave house and wanted to get the patch out before, that dumb
mistake was the result :\
>
> + return -EINVAL;
> + }
>
>
> + err = sss_nss_getsidbyname(name, &sid, &id_type);
> + if (err != 0) {
> + /* Might be a raw string representation of SID,
> + * try converting that before returning an error. */
> + if (sid_to_cifs_sid(ctx, name, csid) == 0)
> + return 0;
> +
>
> ^^^^^^^^ trailing whitespace
>
> Oops, again.. have them highlighted in my editor now ;)
> + ctx_set_error(ctx, strerror(err));
> + return -err;
> + }
>
>
> bye,
> Sumit
>
New patch attached, with these two things fixed.
Code looks good, tests are working as expected, ACK.
bye,
Sumit