Re: [PATCH v2] ieee1275: obdisk driver

[Daniel Kiper]

On Tue, Sep 04, 2018 at 09:45:14AM -0600, Eric Snowberg wrote:
> > On Sep 1, 2018, at 11:10 AM, Daniel Kiper <address@hidden> wrote:

[...]

> > I think that it is correct. If you use one '\' then after shell
> > processing you will get
> >
> > (ieee1275//address@hidden/address@hidden/address@hidden/address@hidden/address@hidden/address@hidden,0)
> >
> > or
> >
> > (ieee1275//address@hidden/address@hidden/address@hidden/address@hidden/address@hidden/address@hidden,0,gpt1)
> >
> > And I suppose that this is not what you want. So, you need two '\'.
> > This way the layer below will get after shell processing
> >
> > (ieee1275//address@hidden/address@hidden/address@hidden/address@hidden/address@hidden/address@hidden,0)
> >
> > or
> >
> > (ieee1275//address@hidden/address@hidden/address@hidden/address@hidden/address@hidden/address@hidden,0,gpt1)
> >
> > Then new driver should get
> >
> > ieee1275//address@hidden/address@hidden/address@hidden/address@hidden/address@hidden/address@hidden,0
> >
> > or
> >
> > ieee1275//address@hidden/address@hidden/address@hidden/address@hidden/address@hidden/address@hidden,0
> >
> > if you really need escaped ',' in the path.
>
> For obdisk devices that are displayed thru the shell, I didn't want
> the escaped ‘,’ in the path.  But you rejected my change that
> substituted it with __ instead.  Therefore we are left with making
> this work with code above obdisk without changing it.
>
> > However, I do not think so.
> > It seems to me that OF expects ',' as ','. Hence, I have a feeling that
> > we can reduce number of escaping/unescaping in the driver.
> >
> > Am I right?
>
> No.
>
> The driver provides the name of the device which is displayed in the
> shell. It must be in a format that will allow it to get back to the
> driver at a later time.  If a ‘,’ is in the name, it must be escaped.
> Otherwise the code above the driver will trim off everything to the
> right of the ‘,’. Then the driver will be sent a device name that does
> not exist.
>
> For example with this device: 
> /address@hidden/address@hidden/LSI,address@hidden/address@hidden,0,gpt3
>
> If the ‘,’ is not escaped properly, either (obdisk or ofdisk) will be
> sent an open to device: /address@hidden/address@hidden/LSI.  The upper level 
> code will
> think everything to the right of the ‘,’ is the partition.  Once the
> driver receives this open, it will fail, since it isn’t a valid
> device.

So, AIUI, the driver has to use device paths with escaped ',', e.g.

  
ieee1275//address@hidden/address@hidden/address@hidden/address@hidden/address@hidden/address@hidden,0

I am OK with that.

Though we should think how to display such paths. We can show them as is
without additional escaping. And right now I think that it is preferred
form. Even GRUB doc shows paths with ',' that way. I know that then
user cannot just copy-and-paste the device path. However, I think that
we should update the doc in relevant place and say that he/she has to
take care about special characters for GRUB2 shell like '\' because shell
will eat them. So, they have to be properly escaped. This way everything
will be in line with common GRUB2 shell escaping rules and we will not
introduce exceptions which may make confusion in the future.

Daniel