[patch] GRUB possible patches

[patch] GRUB possible patches

[George Buranov]

[-- Attachment #1: Type: text/plain, Size: 1577 bytes --]

Hello eveybody.

I am currently start investigation (and fixing the things that don't work
for me =))

So, I got two for today, one is fix, and one is morelike a question, I am
more than sure it's my mistake.

What's the correct way of applying patches to grub2? Am I correct that all I
need is to post the patch to this mailing list and than some guys are
applying them?:)

1) IMHO that's clearly a bug. Actually, I am not sure how did it work before
=)

=== modified file 'disk/efi/efidisk.c'
--- disk/efi/efidisk.c  2010-01-20 08:12:47 +0000
+++ disk/efi/efidisk.c  2010-02-11 10:58:49 +0000
@@ -825,7 +825,7 @@
          if (! disk)
            return 1;

-         if (disk->id == GRUB_DISK_DEVICE_EFIDISK_ID)
+         if (disk->dev && disk->dev->id == GRUB_DISK_DEVICE_EFIDISK_ID)
            {
              struct grub_efidisk_data *d;

2) That's more interesting. I nocticed that when I try to browse CD (ISO
9660 format) all files are ended with ";1". This basically crashed loading
everything (including grub.cfg file). I fixed this by this stupid hack, but
maybe somebody knows where is the clue?

http://picasaweb.google.com/lh/photo/psaYCXsX_M77pdG4_qeFGw?feat=directlink

=== modified file 'fs/iso9660.c'
--- fs/iso9660.c        2010-01-27 03:11:20 +0000
+++ fs/iso9660.c        2010-02-11 11:11:36 +0000
@@ -638,6 +638,9 @@
             if (filename_alloc)
               grub_free (oldname);

+            int hookedFile = grub_strlen (filename);
+            filename[hookedFile - 2] = '\0';
+
             filename_alloc = 1;
           }


Regards,
Georgy

[-- Attachment #2: Type: text/html, Size: 1929 bytes --]

Re: [patch] GRUB possible patches

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 2434 bytes --]

George Buranov wrote:
> Hello eveybody.
>
> I am currently start investigation (and fixing the things that don't
> work for me =))
>
> So, I got two for today, one is fix, and one is morelike a question, I
> am more than sure it's my mistake.
>
> What's the correct way of applying patches to grub2? Am I correct that
> all I need is to post the patch to this mailing list and than some
> guys are applying them?:)
>
> 1) IMHO that's clearly a bug.
It is. Can you write a ChangeLog entry?
> Actually, I am not sure how did it work before =)
It's a rarely used branch of code.
>
> === modified file 'disk/efi/efidisk.c'
> --- disk/efi/efidisk.c  2010-01-20 08:12:47 +0000
> +++ disk/efi/efidisk.c  2010-02-11 10:58:49 +0000
> @@ -825,7 +825,7 @@
>           if (! disk)
>             return 1;
>
> -         if (disk->id == GRUB_DISK_DEVICE_EFIDISK_ID)
> +         if (disk->dev && disk->dev->id == GRUB_DISK_DEVICE_EFIDISK_ID)
>             {
>               struct grub_efidisk_data *d;
>

> 2) That's more interesting. I nocticed that when I try to browse CD
> (ISO 9660 format) all files are ended with ";1". This basically
> crashed loading everything (including grub.cfg file). I fixed this by
> this stupid hack, but maybe somebody knows where is the clue?
>
;1 is so called version. Basically all terminating  ;<number> have to be
stripped but only if filename doesn't come from rockridge or joliet.
Apparently nobody uses grub2 on non-rockridge, non-joliet iso. While
this bug should be fixed using grub from plain iso isn't supported due
to filename length limitation.
> http://picasaweb.google.com/lh/photo/psaYCXsX_M77pdG4_qeFGw?feat=directlink
>
> === modified file 'fs/iso9660.c'
> --- fs/iso9660.c        2010-01-27 03:11:20 +0000
> +++ fs/iso9660.c        2010-02-11 11:11:36 +0000
> @@ -638,6 +638,9 @@
>              if (filename_alloc)
>                grub_free (oldname);
>
> +            int hookedFile = grub_strlen (filename);
> +            filename[hookedFile - 2] = '\0';
> +
>              filename_alloc = 1;
>            }
>
>
> Regards,
> Georgy
> ------------------------------------------------------------------------
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>   


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 293 bytes --]

Re: Re: [patch] GRUB possible patches

[gburanov]

Hello, 

> It is. Can you write a ChangeLog entry?
Done

> ;1 is so called version. Basically all terminating  ;<number> have to be
> stripped but only if filename doesn't come from rockridge or joliet.
> Apparently nobody uses grub2 on non-rockridge, non-joliet iso. While
> this bug should be fixed using grub from plain iso isn't supported due
> to filename length limitation.

I found out that I got versioning even on joliet CD. I created CD using "Small CD Writer". You can try the same. I created patch for that as well, but of course you can skip it.

Regards,
Georgy

PS: My Patch

=== modified file 'ChangeLog'
--- ChangeLog	2010-02-10 19:27:12 +0000
+++ ChangeLog	2010-02-12 14:14:46 +0000
@@ -1,3 +1,8 @@
+2010-02-12  Georgy Buranov <gburanov@gmail.com>
+
+	* disk/efi/efidisk.c (grub_efidisk_get_device_name): Fix bug obtaining device name of the whole disk
+	* fs/iso9660.c (grub_iso9660_iterate_dir): Remove file versions from ISO9660&Joliet file names
+
 2010-02-10  Vladimir Serbinenko  <phcoder@gmail.com>
 
 	Pass SIMPLE framebuffer size in bytes and not 64K blocks.

=== modified file 'disk/efi/efidisk.c'
--- disk/efi/efidisk.c	2010-01-20 08:12:47 +0000
+++ disk/efi/efidisk.c	2010-02-12 14:05:23 +0000
@@ -825,7 +825,7 @@
 	  if (! disk)
 	    return 1;
 
-	  if (disk->id == GRUB_DISK_DEVICE_EFIDISK_ID)
+	  if (disk->dev->id == GRUB_DISK_DEVICE_EFIDISK_ID)
 	    {
 	      struct grub_efidisk_data *d;
 

=== modified file 'fs/iso9660.c'
--- fs/iso9660.c	2010-01-27 03:11:20 +0000
+++ fs/iso9660.c	2010-02-12 14:13:01 +0000
@@ -615,9 +615,6 @@
 	if (!filename)
 	  {
 	    name[dirent.namelen] = '\0';
-	    filename = grub_strrchr (name, ';');
-	    if (filename)
-	      *filename = '\0';
 
 	    if (dirent.namelen == 1 && name[0] == 0)
 	      filename = ".";
@@ -640,6 +637,14 @@
 
             filename_alloc = 1;
           }
+	  
+	 if (!dir->data->rockridge)
+	 {
+	   // On simple ISO 9660 disks and on joliet, we need to remove file version, if any
+	    char* lastSymbol = grub_strrchr (filename, ';');
+	    if (lastSymbol)
+	      *lastSymbol = '\0';
+	 }	  
 
 	if (hook (filename, type, node))
 	  {



--
This message was sent on behalf of gburanov@gmail.com at openSubscriber.com
http://www.opensubscriber.com/message/grub-devel@gnu.org/13415591.html

Re: [patch] GRUB possible patches

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 3065 bytes --]

gburanov@gmail.com wrote:
> Hello, 
>
>   
>> It is. Can you write a ChangeLog entry?
>>     
> Done
>
>   
First part applied
>> ;1 is so called version. Basically all terminating  ;<number> have to be
>> stripped but only if filename doesn't come from rockridge or joliet.
>> Apparently nobody uses grub2 on non-rockridge, non-joliet iso. While
>> this bug should be fixed using grub from plain iso isn't supported due
>> to filename length limitation.
>>     
>
> I found out that I got versioning even on joliet CD. I created CD using "Small CD Writer". You can try the same. I created patch for that as well, but of course you can skip it.
>   
Please don't put unrelated patches into single file. Fast look on the
joliet.c of mkisofs suggests that it's not the case for its joliet. It
seems there is more to it. Can you find some normative documents?
> Regards,
> Georgy
>
> PS: My Patch
>
> === modified file 'ChangeLog'
> --- ChangeLog	2010-02-10 19:27:12 +0000
> +++ ChangeLog	2010-02-12 14:14:46 +0000
> @@ -1,3 +1,8 @@
> +2010-02-12  Georgy Buranov <gburanov@gmail.com>
> +
> +	* disk/efi/efidisk.c (grub_efidisk_get_device_name): Fix bug obtaining device name of the whole disk
> +	* fs/iso9660.c (grub_iso9660_iterate_dir): Remove file versions from ISO9660&Joliet file names
> +
>  2010-02-10  Vladimir Serbinenko  <phcoder@gmail.com>
>  
>  	Pass SIMPLE framebuffer size in bytes and not 64K blocks.
>
> === modified file 'disk/efi/efidisk.c'
> --- disk/efi/efidisk.c	2010-01-20 08:12:47 +0000
> +++ disk/efi/efidisk.c	2010-02-12 14:05:23 +0000
> @@ -825,7 +825,7 @@
>  	  if (! disk)
>  	    return 1;
>  
> -	  if (disk->id == GRUB_DISK_DEVICE_EFIDISK_ID)
> +	  if (disk->dev->id == GRUB_DISK_DEVICE_EFIDISK_ID)
>  	    {
>  	      struct grub_efidisk_data *d;
>  
>
> === modified file 'fs/iso9660.c'
> --- fs/iso9660.c	2010-01-27 03:11:20 +0000
> +++ fs/iso9660.c	2010-02-12 14:13:01 +0000
> @@ -615,9 +615,6 @@
>  	if (!filename)
>  	  {
>  	    name[dirent.namelen] = '\0';
> -	    filename = grub_strrchr (name, ';');
> -	    if (filename)
> -	      *filename = '\0';
>  
>  	    if (dirent.namelen == 1 && name[0] == 0)
>  	      filename = ".";
> @@ -640,6 +637,14 @@
>  
>              filename_alloc = 1;
>            }
> +	  
> +	 if (!dir->data->rockridge)
> +	 {
> +	   // On simple ISO 9660 disks and on joliet, we need to remove file version, if any
>   
We use only C-style comments
> +	    char* lastSymbol = grub_strrchr (filename, ';');
> +	    if (lastSymbol)
> +	      *lastSymbol = '\0';
> +	 }	  
>  
>  	if (hook (filename, type, node))
>  	  {
>
>
>
> --
> This message was sent on behalf of gburanov@gmail.com at openSubscriber.com
> http://www.opensubscriber.com/message/grub-devel@gnu.org/13415591.html
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>
>   


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 293 bytes --]

Re: Re: [patch] GRUB possible patches

[gburanov]

> First part applied
Thanks!

> Please don't put unrelated patches into single file. Fast look on the
> joliet.c of mkisofs suggests that it's not the case for its joliet. It
> seems there is more to it. Can you find some normative documents?

I searched a bit and found the following information:
Doc: http://www.transelectro.ru/technology/standard/joliet_spec.htm
In the documentation: 

"Simply put, SEPARATOR 1 and SEPARATOR 2 shall be expanded to 16-bits".

SEPARATOR 2 is ';' - which is used signify the version number of a file.

About mkisofs - it seems it's just their implementation of joilet, they can use or not use versioning
See this topic
http://www.mail-archive.com/cdwrite@other.debian.org/msg03199.html

> We use only C-style comments
Ok, I will create new patch soon

-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko





--
This message was sent on behalf of gburanov@gmail.com at openSubscriber.com
http://www.opensubscriber.com/message/grub-devel@gnu.org/13437133.html

Re: [patch] GRUB possible patches

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 1061 bytes --]

gburanov@gmail.com wrote:
>> First part applied
>>     
> Thanks!
>
>   
>> Please don't put unrelated patches into single file. Fast look on the
>> joliet.c of mkisofs suggests that it's not the case for its joliet. It
>> seems there is more to it. Can you find some normative documents?
>>     
>
> I searched a bit and found the following information:
> Doc: http://www.transelectro.ru/technology/standard/joliet_spec.htm
> In the documentation: 
>
> "Simply put, SEPARATOR 1 and SEPARATOR 2 shall be expanded to 16-bits".
>
> SEPARATOR 2 is ';' - which is used signify the version number of a file.
>
> About mkisofs - it seems it's just their implementation of joilet, they can use or not use versioning
> See this topic
> http://www.mail-archive.com/cdwrite@other.debian.org/msg03199.html
>
>   
Ok, I agree that it should be fixed but I made and applied a slightly
different fix
>> We use only C-style comments
>>     
> Ok, I will create new patch soon
>
>   


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 293 bytes --]