[linux-cifs-client] [PATCH 2/2] Continued cleanup of open_cred_file and fixed a potential security risk.

[Scott Lovenberg]

>
> > +#define CRED_UNPARSABLE 0
>
>                 ^^^ minor nit...should be "UNPARSEABLE".
>
> Not nitpicking at all, spelling mistakes in code are embarrassing.  I had a
feeling I should have double checked the spelling on that.


>
> > +static int parse_cred_line(char* line, char* target)
> > +{
> > +     if (line == NULL)
> > +             goto parsing_err;
> > +
> > +     /* position target at first char of value */
> > +     target = strchr(line, '=');
> > +     if (!target)
> > +             goto parsing_err;
> > +     target++;
>
> ^^^^
> I don't think this will work. C does pass by value. Here you're passing
> in "target" as a pointer. That value will be updated in the local copy
> of that pointer in parse_cred_line, but caller will not see that you've
> changed it. If you want to do this, you need to pass "target" as a
> pointer to a pointer and fix the references to it accordingly.
>
D'oh.  It's been a few years since I've touched pointers at all.  Easy
enough change.


>
>
> There are also some minor whitespace issues with these patches too
> (trailing whitespace on a few lines).

Yeah, I found out that the formatting problems that I've been having are due
to UTF-8 and interaction between git-format-patch and git-send-email.  I
added a format section to my .gitconfig (and git-format-patch seemed happy),
but I guess I've still got an issue. :/


> Since you'll need to fix the
> above anyway, it would be good to fix those as well.
>
> You might want to run the checkpatch.pl script from the Linux kernel
> sources against these patches and fix the problems it says too. I'd
> like to see this code follow the Linux kernel coding standards for the
> most part.
>
 Will do.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.samba.org/pipermail/linux-cifs-client/attachments/20100423/523c0a24/attachment.html>