Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

2020-03-09 Thread Daniel Kiper
On Fri, Mar 06, 2020 at 02:37:43PM +0100, Vladimir 'phcoder' Serbinenko wrote:
> Le ven. 6 mars 2020 à 13:43, Daniel Kiper  a écrit :
>
> > Re-adding grub-devel@gnu.org...
> >
> > On Fri, Mar 06, 2020 at 12:44:05PM +0100, Vladimir 'phcoder' Serbinenko
> > wrote:
> > > Le ven. 6 mars 2020 à 12:42, Daniel Kiper  a écrit
> > :
> > >
> > > > On Thu, Mar 05, 2020 at 03:22:31PM +0100, Vladimir 'phcoder' Serbinenko
> > > > wrote:
> > > > > Please evaluate size increase for this. In the past passing file and
> > line
> > > > > number to grub_dprintf was a huge source of increased Kern and core
> > size
> > > >
> > > > I think it does not matter much if we stop pretending that we support
> > > > small MBR gaps right now [1]. Does it?
> > > >
> > > There are still a lot of installations that used older tools and never
> > > reformatted. We still care about them
> >
> > If we go that way then we have to care about them by the end of the
> > universe. And this means more and more issues like [1]. If somebody
> > wants to use new GRUB then he/she have to reinstall the machine or
> > something like that. IMO we should not care about users who do not want
> > upgrade their machines or whatnot. Or at least their choices cannot
> > impact GRUB development too much. And I think that this MBR constraint
> > is hindering the project too much at this point. So, as above...
> >
> > Sorry for being blunt...
> >
> It does not. Core doesn't need to have everything and the kitchen sink.
> It's small by design.

I am not saying that. I am saying that limitations of one ancient platform
should not make development of other platforms more difficult.

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

2020-03-06 Thread Vladimir 'phcoder' Serbinenko
> It would be best to use a boot partition so the core.img space is
> reserved by the GPT, but I once wanted to tack i386-pc GRUB onto an
> existing UEFI GPT disk, so I wrote the commands that do what you're
> describing here:
>
> https://github.com/dm0-/installer/blob/master/examples/systems/fitpc.sh#L151-L178
>
Just use gdisk to add a partition into this 1MiB hole.  you only need
to ask it to reduce alignment.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

2020-03-06 Thread Didier Spaier
Le 06/03/2020 à 18:03, David Michael a écrit :
> On Fri, Mar 6, 2020 at 9:02 AM Didier Spaier  wrote:
>> Le 06/03/2020 à 13:43, Daniel Kiper a écrit :
>>> If we go that way then we have to care about them by the end of the
>>> universe. And this means more and more issues like [1]. If somebody
>>> wants to use new GRUB then he/she have to reinstall the machine or
>>> something like that. IMO we should not care about users who do not want
>>> upgrade their machines or whatnot. Or at least their choices cannot
>>> impact GRUB development too much. And I think that this MBR constraint
>>> is hindering the project too much at this point. So, as above...
>>>
>>> Sorry for being blunt...
>>
>> Sorry to be off topic... In case of a GUID partition table, if I
>> understand the UEFI specification[1], as the first usable LBA should be
>> greater than or equal to 34 for a 512 bytes block size or 6 for a
>> 4096 bytes logical block size, it could begin after a gap of 24K.
>>
>> Then, if we assume that the first partition begins @ 1MiB, can't GRUB
>> use the space unused between the first usable LBA and 1MiB instead of
>> a Bios Boot partition, in case of "legacy" booting and a GPT? I ask as
>> then a Bios Boot partition wouldn't be necessary any more.
> 
> It would be best to use a boot partition so the core.img space is
> reserved by the GPT, but I once wanted to tack i386-pc GRUB onto an
> existing UEFI GPT disk, so I wrote the commands that do what you're
> describing here:
> 
> https://github.com/dm0-/installer/blob/master/examples/systems/fitpc.sh#L151-L178
> 
> I may be misremembering, but I think the core.img size was limited to
> around half a megabyte, so it should be safe to write it to the unused
> ~1MiB before the first partition after the GPT.  But yes, using the
> boot partition would probably be for the best if you are formatting a
> new disk.

Thanks for sharing David. This can be useful to allow Legacy booting
off a drive initially partitioned for UEFI booting only.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

2020-03-06 Thread Lennart Sorensen
On Fri, Mar 06, 2020 at 06:10:58PM +0100, Didier Spaier wrote:
> My mistake, I should have written starting at 24 KiB as that is the
> start address of the first usable LBA if I read correctly the UEFI
> specification. Am I wrong?
> 
> Thanks for the heads-up.

I was wrong about 32KB, it is 32 sectors of 512 when that is the block
size plus the MBR and header sectors, so 34 sectors at 512b, and yes
with 4K blocks you hit 24k.  Not sure if bigger sector sizes are likely
to happen at some point.  Maybe 32 or 64KB would be a safer starting
point and being a power of 2, slightly more likely to hit a disk aligned
block size.

-- 
Len Sorensen

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

2020-03-06 Thread Lennart Sorensen
On Fri, Mar 06, 2020 at 11:12:14AM -0600, Bruce Dubbs wrote:
> Just a terminology issue here.  Please don't call it a boot partition. It
> conflicts with those who mount /boot as a separate partition.  Please call
> it a grub partition.

Well the official name is "BIOS boot partition", so I can see why people
tend to call it a boot partition.

-- 
Len Sorensen

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

2020-03-06 Thread Bruce Dubbs

On 3/6/20 11:03 AM, David Michael wrote:

On Fri, Mar 6, 2020 at 9:02 AM Didier Spaier  wrote:

Le 06/03/2020 à 13:43, Daniel Kiper a écrit :

If we go that way then we have to care about them by the end of the
universe. And this means more and more issues like [1]. If somebody
wants to use new GRUB then he/she have to reinstall the machine or
something like that. IMO we should not care about users who do not want
upgrade their machines or whatnot. Or at least their choices cannot
impact GRUB development too much. And I think that this MBR constraint
is hindering the project too much at this point. So, as above...

Sorry for being blunt...


Sorry to be off topic... In case of a GUID partition table, if I
understand the UEFI specification[1], as the first usable LBA should be
greater than or equal to 34 for a 512 bytes block size or 6 for a
4096 bytes logical block size, it could begin after a gap of 24K.

Then, if we assume that the first partition begins @ 1MiB, can't GRUB
use the space unused between the first usable LBA and 1MiB instead of
a Bios Boot partition, in case of "legacy" booting and a GPT? I ask as
then a Bios Boot partition wouldn't be necessary any more.


It would be best to use a boot partition so the core.img space is
reserved by the GPT, 


Just a terminology issue here.  Please don't call it a boot partition. 
It conflicts with those who mount /boot as a separate partition.  Please 
call it a grub partition.


  -- Bruce


but I once wanted to tack i386-pc GRUB onto an

existing UEFI GPT disk, so I wrote the commands that do what you're
describing here:

https://github.com/dm0-/installer/blob/master/examples/systems/fitpc.sh#L151-L178

I may be misremembering, but I think the core.img size was limited to
around half a megabyte, so it should be safe to write it to the unused
~1MiB before the first partition after the GPT.  But yes, using the
boot partition would probably be for the best if you are formatting a
new disk.

Thanks.

David

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel




___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

2020-03-06 Thread Didier Spaier


Le 06/03/2020 à 17:39, Lennart Sorensen a écrit :
> On Fri, Mar 06, 2020 at 05:30:23PM +0100, Didier Spaier wrote:
>> That'd be just one less thing to care about for the Slint installer
>> in case of  an 'automatic' installation, nothing major.
>>
>> Do you confirm that a Bios Boot partition starting at 1 KiB
>> and ending at 1 MiB won't overlap the GPT and is big enough?
> 
> It would absolutely overlap GPT since GPT usually starts at 512 bytes
> and goes up to 32KB or so.

My mistake, I should have written starting at 24 KiB as that is the
start address of the first usable LBA if I read correctly the UEFI
specification. Am I wrong?

Thanks for the heads-up.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

2020-03-06 Thread David Michael
On Fri, Mar 6, 2020 at 9:02 AM Didier Spaier  wrote:
> Le 06/03/2020 à 13:43, Daniel Kiper a écrit :
> > If we go that way then we have to care about them by the end of the
> > universe. And this means more and more issues like [1]. If somebody
> > wants to use new GRUB then he/she have to reinstall the machine or
> > something like that. IMO we should not care about users who do not want
> > upgrade their machines or whatnot. Or at least their choices cannot
> > impact GRUB development too much. And I think that this MBR constraint
> > is hindering the project too much at this point. So, as above...
> >
> > Sorry for being blunt...
>
> Sorry to be off topic... In case of a GUID partition table, if I
> understand the UEFI specification[1], as the first usable LBA should be
> greater than or equal to 34 for a 512 bytes block size or 6 for a
> 4096 bytes logical block size, it could begin after a gap of 24K.
>
> Then, if we assume that the first partition begins @ 1MiB, can't GRUB
> use the space unused between the first usable LBA and 1MiB instead of
> a Bios Boot partition, in case of "legacy" booting and a GPT? I ask as
> then a Bios Boot partition wouldn't be necessary any more.

It would be best to use a boot partition so the core.img space is
reserved by the GPT, but I once wanted to tack i386-pc GRUB onto an
existing UEFI GPT disk, so I wrote the commands that do what you're
describing here:

https://github.com/dm0-/installer/blob/master/examples/systems/fitpc.sh#L151-L178

I may be misremembering, but I think the core.img size was limited to
around half a megabyte, so it should be safe to write it to the unused
~1MiB before the first partition after the GPT.  But yes, using the
boot partition would probably be for the best if you are formatting a
new disk.

Thanks.

David

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

2020-03-06 Thread Lennart Sorensen
On Fri, Mar 06, 2020 at 05:30:23PM +0100, Didier Spaier wrote:
> That'd be just one less thing to care about for the Slint installer
> in case of  an 'automatic' installation, nothing major.
> 
> Do you confirm that a Bios Boot partition starting at 1 KiB
> and ending at 1 MiB won't overlap the GPT and is big enough?

It would absolutely overlap GPT since GPT usually starts at 512 bytes
and goes up to 32KB or so.

-- 
Len Sorensen

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

2020-03-06 Thread Didier Spaier
Le 06/03/2020 à 15:38, Vladimir 'phcoder' Serbinenko a écrit :
> What's the problem with having bios boot partition? You can put it
> exactly in the spot you describe (in fact I do so) and it works as
> well, just additionally it marks this space as occupied

That'd be just one less thing to care about for the Slint installer
in case of  an 'automatic' installation, nothing major.

Do you confirm that a Bios Boot partition starting at 1 KiB
and ending at 1 MiB won't overlap the GPT and is big enough?

Thanks,
Didier


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

2020-03-06 Thread Vladimir 'phcoder' Serbinenko
On Fri, Mar 6, 2020 at 3:02 PM Didier Spaier  wrote:
>
> Le 06/03/2020 à 13:43, Daniel Kiper a écrit :
> > If we go that way then we have to care about them by the end of the
> > universe. And this means more and more issues like [1]. If somebody
> > wants to use new GRUB then he/she have to reinstall the machine or
> > something like that. IMO we should not care about users who do not want
> > upgrade their machines or whatnot. Or at least their choices cannot
> > impact GRUB development too much. And I think that this MBR constraint
> > is hindering the project too much at this point. So, as above...
> >
> > Sorry for being blunt...
>
> Sorry to be off topic... In case of a GUID partition table, if I
> understand the UEFI specification[1], as the first usable LBA should be
> greater than or equal to 34 for a 512 bytes block size or 6 for a
> 4096 bytes logical block size, it could begin after a gap of 24K.
>
> Then, if we assume that the first partition begins @ 1MiB, can't GRUB
> use the space unused between the first usable LBA and 1MiB instead of
> a Bios Boot partition, in case of "legacy" booting and a GPT? I ask as
> then a Bios Boot partition wouldn't be necessary any more.
What's the problem with having bios boot partition? You can put it
exactly in the spot you describe (in fact I do so) and it works as
well, just additionally it marks this space as occupied
>
> I just hope this question is not too silly...
>
> Best regards,
> Didier
>
>
> [1]https://uefi.org/sites/default/files/resources/UEFI_Spec_2_8_A_Feb14.pdf
> § 5.3.1 GPT overview
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

2020-03-06 Thread Didier Spaier
Le 06/03/2020 à 13:43, Daniel Kiper a écrit :
> If we go that way then we have to care about them by the end of the
> universe. And this means more and more issues like [1]. If somebody
> wants to use new GRUB then he/she have to reinstall the machine or
> something like that. IMO we should not care about users who do not want
> upgrade their machines or whatnot. Or at least their choices cannot
> impact GRUB development too much. And I think that this MBR constraint
> is hindering the project too much at this point. So, as above...
> 
> Sorry for being blunt...

Sorry to be off topic... In case of a GUID partition table, if I
understand the UEFI specification[1], as the first usable LBA should be
greater than or equal to 34 for a 512 bytes block size or 6 for a
4096 bytes logical block size, it could begin after a gap of 24K.

Then, if we assume that the first partition begins @ 1MiB, can't GRUB
use the space unused between the first usable LBA and 1MiB instead of
a Bios Boot partition, in case of "legacy" booting and a GPT? I ask as
then a Bios Boot partition wouldn't be necessary any more.

I just hope this question is not too silly...

Best regards,
Didier


[1]https://uefi.org/sites/default/files/resources/UEFI_Spec_2_8_A_Feb14.pdf
§ 5.3.1 GPT overview

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

2020-03-06 Thread Vladimir 'phcoder' Serbinenko
Le ven. 6 mars 2020 à 13:43, Daniel Kiper  a écrit :

> Re-adding grub-devel@gnu.org...
>
> On Fri, Mar 06, 2020 at 12:44:05PM +0100, Vladimir 'phcoder' Serbinenko
> wrote:
> > Le ven. 6 mars 2020 à 12:42, Daniel Kiper  a écrit
> :
> >
> > > On Thu, Mar 05, 2020 at 03:22:31PM +0100, Vladimir 'phcoder' Serbinenko
> > > wrote:
> > > > Please evaluate size increase for this. In the past passing file and
> line
> > > > number to grub_dprintf was a huge source of increased Kern and core
> size
> > >
> > > I think it does not matter much if we stop pretending that we support
> > > small MBR gaps right now [1]. Does it?
> > >
> > There are still a lot of installations that used older tools and never
> > reformatted. We still care about them
>
> If we go that way then we have to care about them by the end of the
> universe. And this means more and more issues like [1]. If somebody
> wants to use new GRUB then he/she have to reinstall the machine or
> something like that. IMO we should not care about users who do not want
> upgrade their machines or whatnot. Or at least their choices cannot
> impact GRUB development too much. And I think that this MBR constraint
> is hindering the project too much at this point. So, as above...
>
> Sorry for being blunt...
>
It does not. Core doesn't need to have everything and the kitchen sink.
It's small by design.

>
> Daniel
>
> [1] https://lists.gnu.org/archive/html/grub-devel/2019-11/msg00025.html
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

2020-03-06 Thread Daniel Kiper
Re-adding grub-devel@gnu.org...

On Fri, Mar 06, 2020 at 12:44:05PM +0100, Vladimir 'phcoder' Serbinenko wrote:
> Le ven. 6 mars 2020 à 12:42, Daniel Kiper  a écrit :
>
> > On Thu, Mar 05, 2020 at 03:22:31PM +0100, Vladimir 'phcoder' Serbinenko
> > wrote:
> > > Please evaluate size increase for this. In the past passing file and line
> > > number to grub_dprintf was a huge source of increased Kern and core size
> >
> > I think it does not matter much if we stop pretending that we support
> > small MBR gaps right now [1]. Does it?
> >
> There are still a lot of installations that used older tools and never
> reformatted. We still care about them

If we go that way then we have to care about them by the end of the
universe. And this means more and more issues like [1]. If somebody
wants to use new GRUB then he/she have to reinstall the machine or
something like that. IMO we should not care about users who do not want
upgrade their machines or whatnot. Or at least their choices cannot
impact GRUB development too much. And I think that this MBR constraint
is hindering the project too much at this point. So, as above...

Sorry for being blunt...

Daniel

[1] https://lists.gnu.org/archive/html/grub-devel/2019-11/msg00025.html

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

2020-03-06 Thread Javier Martinez Canillas
On 3/6/20 12:41 PM, Vladimir 'phcoder' Serbinenko wrote:
> Le ven. 6 mars 2020 à 12:38, Javier Martinez Canillas 
> a écrit :
> 
>> Hello Vladimir,
>>
>> Thanks a lot for your feedback.
>>
>> On 3/5/20 3:22 PM, Vladimir 'phcoder' Serbinenko wrote:
>>> Please evaluate size increase for this. In the past passing file and line
>>> number to grub_dprintf was a huge source of increased Kern and core size
>>>
>>> Le mer. 4 mars 2020 à 13:01, Javier Martinez Canillas <
>> javi...@redhat.com>
>>> a écrit :
>>>
>>
>> For a x86_64-pc build with platform=pc on master with and without the
>> patch:
>>
>> $ find -regex '.*\(mod\|img\|exec\)$' -exec du -c {} + | grep total$
>> 2740total
>>
>> $ find -regex '.*\(mod\|img\|exec\)$' -exec du -c {} + | grep total$
>> 2756total
>>
>> so in this case the increase is a 0.6% in size.
>>
> This is not the comparison we care about. We care about kernel.img size and
> core.img size for several common configs. And in bytes.
> We have only 31K in mbr gap and every byte counts
> 

Fair. Then I'll only do the comparison for pc platform since for EFI it
doesn't matter due the binary being in the ESP.

I built a core.img with the ext2, part_msdos and biosdisk modules which I
think is a pretty common configuration used. So the following command:

$ ./grub-mkimage --directory ./grub-core/ --prefix '(,msdos1)/grub2' --output 
core.img  --format 'i386-pc' --compression 'auto' 'ext2' 'part_msdos' 'biosdisk'

and I got the following sizes in bytes for the two cases:

without the patch: 25977

with the patch: 26350

So I think my comment still applies, it's just a 1.4% increase (373 bytes).

But also as Daniel said, in most cases the gap between the end of the MBR
and the start of the first partition will be at least 1 MiB due partitions
being aligned to a 1 MiB boundary. It shouldn't be an issue for that case.

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

2020-03-06 Thread Vladimir 'phcoder' Serbinenko
Le ven. 6 mars 2020 à 12:38, Javier Martinez Canillas 
a écrit :

> Hello Vladimir,
>
> Thanks a lot for your feedback.
>
> On 3/5/20 3:22 PM, Vladimir 'phcoder' Serbinenko wrote:
> > Please evaluate size increase for this. In the past passing file and line
> > number to grub_dprintf was a huge source of increased Kern and core size
> >
> > Le mer. 4 mars 2020 à 13:01, Javier Martinez Canillas <
> javi...@redhat.com>
> > a écrit :
> >
>
> For a x86_64-pc build with platform=pc on master with and without the
> patch:
>
> $ find -regex '.*\(mod\|img\|exec\)$' -exec du -c {} + | grep total$
> 2740total
>
> $ find -regex '.*\(mod\|img\|exec\)$' -exec du -c {} + | grep total$
> 2756total
>
> so in this case the increase is a 0.6% in size.
>
This is not the comparison we care about. We care about kernel.img size and
core.img size for several common configs. And in bytes.
We have only 31K in mbr gap and every byte counts
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

2020-03-06 Thread Daniel Kiper
On Thu, Mar 05, 2020 at 03:22:31PM +0100, Vladimir 'phcoder' Serbinenko wrote:
> Please evaluate size increase for this. In the past passing file and line
> number to grub_dprintf was a huge source of increased Kern and core size

I think it does not matter much if we stop pretending that we support
small MBR gaps right now [1]. Does it?

Daniel

[1] https://lists.gnu.org/archive/html/grub-devel/2020-03/msg00059.html

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

2020-03-06 Thread Javier Martinez Canillas
Hello Vladimir,

Thanks a lot for your feedback.

On 3/5/20 3:22 PM, Vladimir 'phcoder' Serbinenko wrote:
> Please evaluate size increase for this. In the past passing file and line
> number to grub_dprintf was a huge source of increased Kern and core size
> 
> Le mer. 4 mars 2020 à 13:01, Javier Martinez Canillas 
> a écrit :
> 

For a x86_64-pc build with platform=pc on master with and without the patch:

$ find -regex '.*\(mod\|img\|exec\)$' -exec du -c {} + | grep total$
2740total

$ find -regex '.*\(mod\|img\|exec\)$' -exec du -c {} + | grep total$
2756total

so in this case the increase is a 0.6% in size.

And doing the same in a x86_64 with platform=efi build:

$ find -regex '.*\(mod\|img\|exec\)$' -exec du -c {} + | grep total$
4296total

$ find -regex '.*\(mod\|img\|exec\)$' -exec du -c {} + | grep total$
4356total

The size increase it's a little bigger but just a 1.4%.

You are right that there's a size increase but I wouldn't call it huge and
I think that's a good trade off for the convenience of having that info.

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

2020-03-05 Thread Vladimir 'phcoder' Serbinenko
Please evaluate size increase for this. In the past passing file and line
number to grub_dprintf was a huge source of increased Kern and core size

Le mer. 4 mars 2020 à 13:01, Javier Martinez Canillas 
a écrit :

> From: Peter Jones 
>
> Add file and line to grub_error() output to make troubleshooting easier.
>
> Signed-off-by: Peter Jones 
> Signed-off-by: Javier Martinez Canillas 
> ---
>
>  grub-core/kern/err.c | 13 +++--
>  include/grub/err.h   |  5 -
>  2 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/grub-core/kern/err.c b/grub-core/kern/err.c
> index 53c734de70e..aebfe0cf839 100644
> --- a/grub-core/kern/err.c
> +++ b/grub-core/kern/err.c
> @@ -33,15 +33,24 @@ static struct grub_error_saved
> grub_error_stack_items[GRUB_ERROR_STACK_SIZE];
>  static int grub_error_stack_pos;
>  static int grub_error_stack_assert;
>
> +#ifdef grub_error
> +#undef grub_error
> +#endif
> +
>  grub_err_t
> -grub_error (grub_err_t n, const char *fmt, ...)
> +grub_error (grub_err_t n, const char *file, const int line, const char
> *fmt, ...)
>  {
>va_list ap;
> +  int m;
>
>grub_errno = n;
>
> +  m = grub_snprintf (grub_errmsg, sizeof (grub_errmsg), "%s:%d:", file,
> line);
> +  if (m < 0)
> +m = 0;
> +
>va_start (ap, fmt);
> -  grub_vsnprintf (grub_errmsg, sizeof (grub_errmsg), _(fmt), ap);
> +  grub_vsnprintf (grub_errmsg + m, sizeof (grub_errmsg) - m, _(fmt), ap);
>va_end (ap);
>
>return n;
> diff --git a/include/grub/err.h b/include/grub/err.h
> index 24ba9f5f592..b68bbec3c72 100644
> --- a/include/grub/err.h
> +++ b/include/grub/err.h
> @@ -85,7 +85,10 @@ struct grub_error_saved
>  extern grub_err_t EXPORT_VAR(grub_errno);
>  extern char EXPORT_VAR(grub_errmsg)[GRUB_MAX_ERRMSG];
>
> -grub_err_t EXPORT_FUNC(grub_error) (grub_err_t n, const char *fmt, ...);
> +grub_err_t EXPORT_FUNC(grub_error) (grub_err_t n, const char *file, const
> int line, const char *fmt, ...);
> +
> +#define grub_error(n, fmt, ...) grub_error (n, __FILE__, __LINE__, fmt,
> ##__VA_ARGS__)
> +
>  void EXPORT_FUNC(grub_fatal) (const char *fmt, ...) __attribute__
> ((noreturn));
>  void EXPORT_FUNC(grub_error_push) (void);
>  int EXPORT_FUNC(grub_error_pop) (void);
> --
> 2.24.1
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 08/12] kern: Make grub_error() more verbose

2020-03-05 Thread Daniel Kiper
On Wed, Mar 04, 2020 at 12:58:47PM +0100, Javier Martinez Canillas wrote:
> From: Peter Jones 
>
> Add file and line to grub_error() output to make troubleshooting easier.
>
> Signed-off-by: Peter Jones 
> Signed-off-by: Javier Martinez Canillas 

Reviewed-by: Daniel Kiper 

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel