Re: [PATCH] 9p: unsigned/signed wrap in p9/unix modes.

[Geyslan GregÃrio Bem]

2013/10/20 Eric Van Hensbergen <ericvh@xxxxxxxxx>:
> Please resubmit a clean patch which includes the check of sscanf for exactly
> the correct number of arguments and handles errors properly in other cases.
> That last bit may be a bit problematic since right now the only errors are
> prints and we seem to be otherwise silently failing.  Of course, looks like
> nothing else is checking return values from that function for error.  We
> could set rdev to an ERR_PTR(-EIO), and then go through and do a check in
> all the places which matter (looks like there are a few places where rdev
> just gets discarded -- might even be a good idea to not parse rdev unless we
> need to by passing NULL to p9mode2unixmode.
>
> All in all, its a corner case which is only likely with a broken server, but
> the full clean up would seem to be:
>   a) switch to u32's
>   b) pass NULL when rdev just gets discarded and don't bother parsing when
> it is
>   c) check the sscanf return validity
>   d) on error set ERR_PTR in rdev and check on return before proceeding
>
> That's a lot of cleanup, I'll add it to my work queue if you don't have time
> to rework your patch.
>

Eric, I would like to try with your guidance.

> For the other patches, anyone you didn't see a response from me on today is
> being pulled into my for-next queue.  Thanks for the cleanups.
>
>       -eric

Thanks for accept them.

>
>
>
>
> On Mon, Oct 7, 2013 at 7:18 PM, Geyslan GregÃrio Bem <geyslan@xxxxxxxxx>
> wrote:
>>
>> Joe,
>>
>> Nice, I'll wait their reply, there are other p9 patches that I have
>> already sent and am awaiting Eric's.
>>
>> Thank you again.
>>
>> Geyslan GregÃrio Bem
>> hackingbits.com
>>
>>
>> 2013/10/7 Joe Perches <joe@xxxxxxxxxxx>:
>> > On Mon, 2013-10-07 at 21:09 -0300, Geyslan GregÃrio Bem wrote:
>> >> Joe,
>> >>
>> >> Thank you for reply.
>> >>
>> >> What do you think about:
>> >>
>> >>                  strncpy(ext, stat->extension, sizeof(ext));
>> >> +                 if (sscanf(ext, "%c %u %u", &type, &major, &minor) <
>> >> 3) {
>> >> +                                  p9_debug(P9_DEBUG_ERROR,
>> >> +                                  "It's necessary define type, major
>> >> and minor values when using P9_DMDEVICE");
>> >> +                                  return res;
>> >> +                 }
>> >>                  switch (type) {
>> >>                  case 'c':
>> >>                          res |= S_IFCHR;
>> >>                          break;
>> >> ...
>> >>                  *rdev = MKDEV(major, minor);
>> >
>> > I think the plan 9 folk should figure out what's right.
>> >
>> >
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/