svn commit: r224150 - head/sbin/fdisk

[Ryan Stone]

On Sun, Jul 17, 2011 at 7:09 PM, Bruce Evans <brde at optusnet.com.au> wrote:
> There was no need to further break the style.

Ack.  Should have caught that.  Will fix.

>>
>> @@ -990,7 +990,7 @@ parse_config_line(char *line, CMD *comma
>>            if (isalpha(*cp))
>>                command->args[command->n_args].argtype = *cp++;
>>            end = NULL;
>> -           command->args[command->n_args].arg_val = strtol(cp, &end, 0);
>> +           command->args[command->n_args].arg_val = strtoul(cp, &end, 0);
>>            if (cp == end || (!isspace(*end) && *end != '\0')) {
>>                char ch;
>>                end = cp;
>>
>
> Also, arg_val is never used as a u_long.  We risk truncation when it is
> blindly assigned to variables whose type is never u_long.  It is mostly
> assigned to variables of type uint ("uint" is an archaic SysV spelling
> of the BSD u_int which is a spelling of unsigned int.  This style bug
> was imported from Mach and never fixed).  Since uint is unsigned and ints
> have 32 bits, things now work up to UINT32_MAX but no further.  arg_val
> is also blindly assigned to a variable of type int, the partition number,
> and to a variable of type u_char (the partition type), but since these
> values nonsense unless they are small, there is no problem with overflow
> near INT32_MAX or UINT32_MAX.

Hm, I was under the impression that fdisk was checking whether the
size parameter fit within in 32-bits before putting it in the MBR, but
I seem to have been mistaken.  In that case, I think that the right
fix is to make arg_val a u_int for now, which will still allow slices
with more than 2^31 sectors to be specified.  Truncation is still an
issue but that's not a new bug.