On Sat, Dec 11, 2010 at 1:46 PM, maximilian attems <maks@stro.at> wrote:
> thank you for submitting, I had seen it and had a question to the patch.
>
>> === modified file 'scripts/functions'
>> --- scripts/functions 2010-10-04 22:34:10 +0000
>> +++ scripts/functions 2010-11-27 02:56:12 +0000
>> @@ -364,8 +364,8 @@
>> return
>> ;;
>> [0-9]*:[0-9]*)
>> - minor=${1#*:}
>> - major=${1%:*}
>> + minor=$(echo ${1#*:} | sed -e 's/^0*//')
>> + major=$(echo ${1%:*} | sed -e 's/^0*//')
>
> I had wondered why you remove leading nulls, but now thanks
> to your aboves desc I do not understand.
>
> Now the trouble is the implementation, please use shell parameter
> expansion. sed assumes busybox in the initramfs, which might not be
> with the case with BUSYBOX=no for small initramfs or mem constraint
> boxes. something like ${minor#0}, not sure if we need to account for more
> then 2 prefixed zeroes, in this case shell guru's can give a hint.
I see. I wasn't aware of BUSYBOX=no. But looking at this again, I
think I actually want to take a different approach in order to
maintain the current semantics of setting root=X:Y.
In particular, if the user passes in a value that has a prefixed 0,
that will get passed directly to mknod, which would treat the number
as octal. So root=010:0 would actually create a device with major
number 8. Instead of trying to resolve this directly, we can use the
shell's arithmetic expansion to get a decimal number back.
>> ;;
>> [A-Fa-f0-9]*)
>> value=$(( 0x${1} ))
>> @@ -377,8 +377,7 @@
>> ;;
>> esac
>>
>> - mknod -m 600 /dev/root b ${major} ${minor}
>> - ROOT=/dev/root
>> + ROOT=/dev/block/${major}:${minor}
>> }
>>
>> # Parameter: device node to check
>
> this assumes udev running in initramfs for the symlink.
> together with devtmpfs and force loading one can built one without,
> but I agree that this may now be very special case.
I think that's reasonable to address. udev is only priority:
important, so it's not inconceivable to imagine initramfs's without
it. It's easy enough to check for it and fall back on the old behavior
if udev isn't there.
> thank you for the feedback.
Thank you for yours. I appreciate the consideration of these edge
cases; I probably wouldn't have come up with them myself.
I've attached a new patch that incorporates your suggestions.
- Evan
Attachment:
udev-friendly-parse_numeric.diff
Description: Binary data