Hollis Blanchard <[EMAIL PROTECTED]> writes:
Hi Hollis,
Sorry for the late reply. For some reason I did not see this email.
>> You could get the property length and use that. It would be clearer.
>
> All these stack-based get_property calls are an accident waiting to
> happen, but you've advocated them until now. :-P Instead, we need a
> wrapper function that returns a malloced buffer of the proper
> size. That's what you're suggesting open-coding here anyways.
Using malloc is way better.
> However I would rather not do that right now. The few developers
> (myself included) have other things going on, and there are still
> significant functionality gaps. Right now I don't want to spend time
> developing patches that produce lots of code churn and no user-visible
> improvement. Others are more than welcome to, of course. :)
Sure, I don't have a big problem with this in this case. But please
don't do that all the time because that will just make a crap out of
GRUB, which is something I don't want.
>> Device arguments are like the partition number or so?
>
> Yes. OF "node names" contain two parts, the device path and the device
> arguments. The device arguments are everything following a colon, and
> the syntax of the arguments is device-specific (but by convention they
> are separated by commas).
>
> In the case of a node name like "disk:6,grubof", "6,grubof" are the
> device arguments.
Ok.
> I used the term "device specifier" incorrectly the comments; I will
> correct that. (IEEE1275 defines its terms at the beginning, which is
> quite helpful...)
Great. :)
> As a separate issue, I *believe* that bootpath should always be a
> fully-expanded node name, and so always contain a colon, but that
> depends on well-behaved firmware. Accordingly, I don't think a sanity
> check here is unreasonable.
When dealing with some open firmware implementations, it is better to
be paranoid. ;)
> However, as we can see experimentally, at least Apple firmware
> includes the device arguments in bootpath. My briQ's firmware does
> not, at least for netbooting:
> ok boot net:,grubof.chrp
> grub> suspend
> ok dev /chosen
> ok .properties
> ...
> bootargs "briqboot"
> bootpath "/[EMAIL PROTECTED]/[EMAIL PROTECTED]"
So we don't get an alias... hmm.
Doesn't this cause any problems?
> ... where "briqboot" is the contents of the the boot-file environment
> variable. This is likely the behavior Sven said was broken.
Ok..
>>> +char *
>>> +grub_ieee1275_get_dev_encoding (const char *path)
>>> +{
>>> + char *device = grub_ieee1275_get_devname (path);
>>> + char *partition = grub_ieee1275_parse_args (path,
>>> GRUB_PARSE_PARTITION);
>>> + char *encoding;
>>> +
>>> + if (partition)
>>> + {
>>> + unsigned int partno = grub_strtoul (partition, 0, 0);
>>> + partno--; /* GRUB partition numbering is 0-based. */
>>
>> Right. But how can you be sure both match?
>
> Eh? OF partition numbers are 1-based. To convert to GRUB's 0-based
> numbering, we subtract one. How could that not "match"?
Because not in all cases GRUB and the firmware will count partitions
the same way. A good example is the PC partition map. In linux
primary partitions are numbers from 1 to 4, extended partitions are
numbered from 5 (IIRC). One other way to count these partitions is
just by starting counting from 1.
This is just an example. There are a lot of partition table layouts
and many ways to interpret partition numbers. I can imagine GRUB
does not always work the same as a specific firmware implementation
all the time.
>>> +
>>> + /* XXX Assume partno will only require two bytes? */
>>> + encoding = grub_malloc (grub_strlen (device) + 5);
>>
>> Can't you just calculate this? Otherwise you can better allocate a
>> few bytes too much.
>
> How exactly would you like to calculate what sprintf will do before it
> does it? We have no snprintf (with "sophisticated" return values), and
> I don't think it's worth the effort either.
>
> I will change the 5 to an 8 if it makes you happy, though I would be
> very surprised to see a disk with partitions in even the (base 10)
> triple digits.
Please do.
--
Marco
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
