[PATCH] iso9660 UUID support by using the creation date/time

[PATCH] iso9660 UUID support by using the creation date/time

[Felix Zielcke]

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

Here's a patch which implements UUID support for the iso9660 filesystem,
by using the creation date in the `superblock'.
The specs say that it's allowed to contain only zeros but I think this
shouldn't be a big problem.


-- 
Felix Zielcke

[-- Attachment #2: iso9660_uuid.patch --]
[-- Type: text/x-patch, Size: 4410 bytes --]

2008-08-30  Felix Zielcke  <fzielcke@z-51.de>

	* fs/iso9660.c (grub_iso9660_date): New structure.
	(grub_iso9660_primary_voldesc): Add `struct grub_iso9660_date' member.
	(grub_iso9660_uuid): New function.

Index: fs/iso9660.c
===================================================================
--- fs/iso9660.c	(Revision 1836)
+++ fs/iso9660.c	(Arbeitskopie)
@@ -67,6 +67,18 @@ struct grub_iso9660_dir
   grub_uint8_t namelen;
 } __attribute__ ((packed));
 
+struct grub_iso9660_date
+{
+  grub_uint8_t year[4];
+  grub_uint8_t month[2];
+  grub_uint8_t day[2];
+  grub_uint8_t hour[2];
+  grub_uint8_t minute[2];
+  grub_uint8_t second[2];
+  grub_uint8_t hundredth[2];
+  grub_uint8_t offset;
+}__attribute__ ((packed));
+
 /* The primary volume descriptor.  Only little endian is used.  */
 struct grub_iso9660_primary_voldesc
 {
@@ -81,6 +93,8 @@ struct grub_iso9660_primary_voldesc
   grub_uint32_t path_table;
   grub_uint8_t unused5[12];
   struct grub_iso9660_dir rootdir;
+  grub_uint8_t unused6[641];
+  struct grub_iso9660_date created;
 } __attribute__ ((packed));
 
 /* A single entry in the path table.  */
@@ -812,8 +826,45 @@ grub_iso9660_label (grub_device_t device
   return grub_errno;
 }
 
-\f
 
+static grub_err_t
+grub_iso9660_uuid (grub_device_t device, char **uuid)
+{
+  struct grub_iso9660_data *data;
+  grub_disk_t disk = device->disk;
+
+#ifndef GRUB_UTIL
+  grub_dl_ref (my_mod);
+#endif
+
+  data = grub_iso9660_mount (disk);
+  if (data)
+    {
+      *uuid = grub_malloc (sizeof (struct grub_iso9660_date) + sizeof ('\0'));
+      grub_sprintf (*uuid, "%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%02u",
+		    data->voldesc.created.year[0], data->voldesc.created.year[1], 
+		    data->voldesc.created.year[2], data->voldesc.created.year[3],
+		    data->voldesc.created.month[0], data->voldesc.created.month[1],
+		    data->voldesc.created.day[0], data->voldesc.created.day[1],
+		    data->voldesc.created.hour[0], data->voldesc.created.hour[1],
+		    data->voldesc.created.minute[0], data->voldesc.created.minute[1],
+		    data->voldesc.created.second[0], data->voldesc.created.second[2],
+		    data->voldesc.created.hundredth[0], data->voldesc.created.hundredth[1],
+		    data->voldesc.created.offset);
+    }
+  else
+    *uuid = NULL;
+
+#ifndef GRUB_UTIL
+  grub_dl_unref (my_mod);
+#endif
+
+  grub_free (data);
+
+  return grub_errno;
+}
+
+\f
 static struct grub_fs grub_iso9660_fs =
   {
     .name = "iso9660",
@@ -822,6 +873,7 @@ static struct grub_fs grub_iso9660_fs =
     .read = grub_iso9660_read,
     .close = grub_iso9660_close,
     .label = grub_iso9660_label,
+    .uuid = grub_iso9660_uuid,
     .next = 0
   };
 
Index: config.h.in
===================================================================
--- config.h.in	(Revision 1836)
+++ config.h.in	(Arbeitskopie)
@@ -109,17 +109,48 @@
 /* Define to 1 if you have the ANSI C header files. */
 #undef STDC_HEADERS
 
-/* Define to 1 if your processor stores words with the most significant byte
-   first (like Motorola and SPARC, unlike Intel and VAX). */
-#undef WORDS_BIGENDIAN
+/* Define WORDS_BIGENDIAN to 1 if your processor stores words with the most
+   significant byte first (like Motorola and SPARC, unlike Intel and VAX). */
+#if defined __BIG_ENDIAN__
+# define WORDS_BIGENDIAN 1
+#elif ! defined __LITTLE_ENDIAN__
+# undef WORDS_BIGENDIAN
+#endif
 
 /* Number of bits in a file offset, on hosts where this is settable. */
 #undef _FILE_OFFSET_BITS
 
+/* Define for large files, on AIX-style hosts. */
+#undef _LARGE_FILES
+
+/* Define to 1 if on MINIX. */
+#undef _MINIX
+
+/* Define to 2 if the system does not provide POSIX.1 features except with
+   this defined. */
+#undef _POSIX_1_SOURCE
+
+/* Define to 1 if you need to in order for `stat' and other things to work. */
+#undef _POSIX_SOURCE
+
+/* Enable extensions on AIX 3, Interix.  */
+#ifndef _ALL_SOURCE
+# undef _ALL_SOURCE
+#endif
 /* Enable GNU extensions on systems that have them.  */
 #ifndef _GNU_SOURCE
 # undef _GNU_SOURCE
 #endif
+/* Enable threading extensions on Solaris.  */
+#ifndef _POSIX_PTHREAD_SEMANTICS
+# undef _POSIX_PTHREAD_SEMANTICS
+#endif
+/* Enable extensions on HP NonStop.  */
+#ifndef _TANDEM_SOURCE
+# undef _TANDEM_SOURCE
+#endif
+/* Enable general extensions on Solaris.  */
+#ifndef __EXTENSIONS__
+# undef __EXTENSIONS__
+#endif
 
-/* Define for large files, on AIX-style hosts. */
-#undef _LARGE_FILES

Re: [PATCH] iso9660 UUID support by using the creation date/time

[Felix Zielcke]

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

Am Samstag, den 30.08.2008, 22:15 +0200 schrieb Felix Zielcke:
> Here's a patch which implements UUID support for the iso9660 filesystem,
> by using the creation date in the `superblock'.
> The specs say that it's allowed to contain only zeros but I think this
> shouldn't be a big problem.


I just got another idea, make GCC warnings more visible.
It did work fine with grub-emu and the CD I tested with.

I have removed now the offset, grub_sprintf doestn't support %hhu
format, and as far as I understand it's stored like a normal `signed
int' just with 8bit size i.e. `signed char' but not a ASCII digit.
Without it, it should be still unique enough.

So better try it out with this attached one :)

-- 
Felix Zielcke

[-- Attachment #2: iso9660_uuid2.patch --]
[-- Type: text/x-patch, Size: 2610 bytes --]

2008-08-30  Felix Zielcke  <fzielcke@z-51.de>

	* fs/iso9660.c (grub_iso9660_date): New structure.
	(grub_iso9660_primary_voldesc): Add `grub_iso9660_date' member.
	(grub_iso9660_uuid): New function.

Index: fs/iso9660.c
===================================================================
--- fs/iso9660.c	(Revision 1836)
+++ fs/iso9660.c	(Arbeitskopie)
@@ -67,6 +67,18 @@ struct grub_iso9660_dir
   grub_uint8_t namelen;
 } __attribute__ ((packed));
 
+struct grub_iso9660_date
+{
+  grub_uint8_t year[4];
+  grub_uint8_t month[2];
+  grub_uint8_t day[2];
+  grub_uint8_t hour[2];
+  grub_uint8_t minute[2];
+  grub_uint8_t second[2];
+  grub_uint8_t hundredth[2];
+  grub_uint8_t offset;
+}__attribute__ ((packed));
+
 /* The primary volume descriptor.  Only little endian is used.  */
 struct grub_iso9660_primary_voldesc
 {
@@ -81,6 +93,8 @@ struct grub_iso9660_primary_voldesc
   grub_uint32_t path_table;
   grub_uint8_t unused5[12];
   struct grub_iso9660_dir rootdir;
+  grub_uint8_t unused6[641];
+  struct grub_iso9660_date created;
 } __attribute__ ((packed));
 
 /* A single entry in the path table.  */
@@ -812,8 +826,44 @@ grub_iso9660_label (grub_device_t device
   return grub_errno;
 }
 
-\f
 
+static grub_err_t
+grub_iso9660_uuid (grub_device_t device, char **uuid)
+{
+  struct grub_iso9660_data *data;
+  grub_disk_t disk = device->disk;
+
+#ifndef GRUB_UTIL
+  grub_dl_ref (my_mod);
+#endif
+
+  data = grub_iso9660_mount (disk);
+  if (data)
+    {
+      *uuid = grub_malloc (sizeof ("xxxxxxxxxxxxxxxx") + sizeof ('\0'));
+      grub_sprintf (*uuid, "%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c",
+		    data->voldesc.created.year[0], data->voldesc.created.year[1], 
+		    data->voldesc.created.year[2], data->voldesc.created.year[3],
+		    data->voldesc.created.month[0], data->voldesc.created.month[1],
+		    data->voldesc.created.day[0], data->voldesc.created.day[1],
+		    data->voldesc.created.hour[0], data->voldesc.created.hour[1],
+		    data->voldesc.created.minute[0], data->voldesc.created.minute[1],
+		    data->voldesc.created.second[0], data->voldesc.created.second[1],
+		    data->voldesc.created.hundredth[0], data->voldesc.created.hundredth[1])
+    }
+  else
+    *uuid = NULL;
+
+#ifndef GRUB_UTIL
+  grub_dl_unref (my_mod);
+#endif
+
+  grub_free (data);
+
+  return grub_errno;
+}
+
+\f
 static struct grub_fs grub_iso9660_fs =
   {
     .name = "iso9660",
@@ -822,6 +872,7 @@ static struct grub_fs grub_iso9660_fs =
     .read = grub_iso9660_read,
     .close = grub_iso9660_close,
     .label = grub_iso9660_label,
+    .uuid = grub_iso9660_uuid,
     .next = 0
   };
 

Re: [PATCH] iso9660 UUID support by using the creation date/time

[Robert Millan]

On Sat, Aug 30, 2008 at 10:15:36PM +0200, Felix Zielcke wrote:
> Here's a patch which implements UUID support for the iso9660 filesystem,
> by using the creation date in the `superblock'.

Nice work :-)

> The specs say that it's allowed to contain only zeros but I think this
> shouldn't be a big problem.

I think this could indeed be a problem if it leads to collisions.  If "all
zeroes" is detected, one could rise an error in uuid() function to prevent the
caller from taking the value into consideration.

> +      *uuid = grub_malloc (sizeof (struct grub_iso9660_date) + sizeof ('\0'));
> +      grub_sprintf (*uuid, "%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%02u",
> +		    data->voldesc.created.year[0], data->voldesc.created.year[1], 
> +		    data->voldesc.created.year[2], data->voldesc.created.year[3],
> +		    data->voldesc.created.month[0], data->voldesc.created.month[1],
> +		    data->voldesc.created.day[0], data->voldesc.created.day[1],
> +		    data->voldesc.created.hour[0], data->voldesc.created.hour[1],
> +		    data->voldesc.created.minute[0], data->voldesc.created.minute[1],
> +		    data->voldesc.created.second[0], data->voldesc.created.second[2],
> +		    data->voldesc.created.hundredth[0], data->voldesc.created.hundredth[1],
> +		    data->voldesc.created.offset);

Since the string contains human-readable information, may I suggest separating
it with dashes to make it easier to comprehend?

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH] iso9660 UUID support by using the creation date/time

[Felix Zielcke]

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

Hello list,

Am Sonntag, den 31.08.2008, 15:47 +0200 schrieb Robert Millan:

> Nice work :-)

By the way the TAB key on emacs is really nice you even don't need to
care about moving the cursor to the beginning of the line :)

> > The specs say that it's allowed to contain only zeros but I think this
> > shouldn't be a big problem.
> 
> I think this could indeed be a problem if it leads to collisions.  If "all
> zeroes" is detected, one could rise an error in uuid() function to prevent the
> caller from taking the value into consideration.

I have now used grub_error (GRUB_ERR_BAD_NUMBER)

But maybe it would be better to make a new type for this, though I don't
have yet an idea how to call it.
Suggestions please :)


> Since the string contains human-readable information, may I suggest separating
> it with dashes to make it easier to comprehend?

Good idea, I should have done that first that would make it even for me a bit easier.
I did it now even more human-readable for the `sizeof ()' 

-- 
Felix Zielcke

[-- Attachment #2: iso9660_uuid.patch.3 --]
[-- Type: text/plain, Size: 3345 bytes --]

2008-08-31  Felix Zielcke  <fzielcke@z-51.de>

	* fs/iso9660.c (grub_iso9660_date): New structure.
	(grub_iso9660_primary_voldesc): Add `grub_iso9660_date' member.
	(grub_iso9660_uuid): New function.


Index: fs/iso9660.c
===================================================================
--- fs/iso9660.c	(Revision 1839)
+++ fs/iso9660.c	(Arbeitskopie)
@@ -67,6 +67,18 @@ struct grub_iso9660_dir
   grub_uint8_t namelen;
 } __attribute__ ((packed));
 
+struct grub_iso9660_date
+{
+  grub_uint8_t year[4];
+  grub_uint8_t month[2];
+  grub_uint8_t day[2];
+  grub_uint8_t hour[2];
+  grub_uint8_t minute[2];
+  grub_uint8_t second[2];
+  grub_uint8_t hundredth[2];
+  grub_uint8_t offset;
+}__attribute__ ((packed));
+
 /* The primary volume descriptor.  Only little endian is used.  */
 struct grub_iso9660_primary_voldesc
 {
@@ -81,6 +93,8 @@ struct grub_iso9660_primary_voldesc
   grub_uint32_t path_table;
   grub_uint8_t unused5[12];
   struct grub_iso9660_dir rootdir;
+  grub_uint8_t unused6[641];
+  struct grub_iso9660_date created;
 } __attribute__ ((packed));
 
 /* A single entry in the path table.  */
@@ -812,8 +826,59 @@ grub_iso9660_label (grub_device_t device
   return grub_errno;
 }
 
-\f
 
+static grub_err_t
+grub_iso9660_uuid (grub_device_t device, char **uuid)
+{
+  struct grub_iso9660_data *data;
+  grub_disk_t disk = device->disk;
+
+#ifndef GRUB_UTIL
+  grub_dl_ref (my_mod);
+#endif
+
+  data = grub_iso9660_mount (disk);
+  if (data)
+    {
+      if (! data->voldesc.created.year[0] && ! data->voldesc.created.year[1]
+	  && ! data->voldesc.created.year[2] && ! data->voldesc.created.year[3]
+	  && ! data->voldesc.created.month[0] && ! data->voldesc.created.month[1]
+	  && ! data->voldesc.created.day[0] && ! data->voldesc.created.day[1]
+	  && ! data->voldesc.created.hour[0] && ! data->voldesc.created.hour[1]
+	  && ! data->voldesc.created.minute[0] && ! data->voldesc.created.minute[1]
+	  && ! data->voldesc.created.second[0] && ! data->voldesc.created.second[1]
+	  && ! data->voldesc.created.hundredth[0] && ! data->voldesc.created.hundredth[1])
+	{
+	  grub_error (GRUB_ERR_BAD_NUMBER, "No creation date in filesystem to generate UUID.");
+	  *uuid = NULL;
+	}
+      else 
+	{
+	  *uuid = grub_malloc (sizeof ("YYYY-MM-DD-HH-mm-ss-hh") + sizeof ('\0'));
+	  grub_sprintf (*uuid, "%c%c%c%c-%c%c-%c%c-%c%c-%c%c-%c%c-%c%c",
+			data->voldesc.created.year[0], data->voldesc.created.year[1], 
+			data->voldesc.created.year[2], data->voldesc.created.year[3],
+			data->voldesc.created.month[0], data->voldesc.created.month[1],
+			data->voldesc.created.day[0], data->voldesc.created.day[1],
+			data->voldesc.created.hour[0], data->voldesc.created.hour[1],
+			data->voldesc.created.minute[0], data->voldesc.created.minute[1],
+			data->voldesc.created.second[0], data->voldesc.created.second[1],
+			data->voldesc.created.hundredth[0], data->voldesc.created.hundredth[1]);
+	}
+    }
+  else
+    *uuid = NULL;
+
+#ifndef GRUB_UTIL
+	grub_dl_unref (my_mod);
+#endif
+
+  grub_free (data);
+
+  return grub_errno;
+}
+
+\f
 static struct grub_fs grub_iso9660_fs =
   {
     .name = "iso9660",
@@ -822,6 +887,7 @@ static struct grub_fs grub_iso9660_fs =
     .read = grub_iso9660_read,
     .close = grub_iso9660_close,
     .label = grub_iso9660_label,
+    .uuid = grub_iso9660_uuid,
     .next = 0
   };
 

Re: [PATCH] iso9660 UUID support by using the creation date/time

[Javier Martín]

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

El dom, 31-08-2008 a las 16:27 +0200, Felix Zielcke escribió:
> > > The specs say that it's allowed to contain only zeros but I think this
> > > shouldn't be a big problem.
> > 
> > I think this could indeed be a problem if it leads to collisions.  If "all
> > zeroes" is detected, one could rise an error in uuid() function to prevent the
> > caller from taking the value into consideration.
> 
> I have now used grub_error (GRUB_ERR_BAD_NUMBER)
> 
> But maybe it would be better to make a new type for this, though I don't
> have yet an idea how to call it.
> Suggestions please :)
What about:
 * BAD_FS looks the most contextual here, but if the ISO9660 standard
allows filesystems to "lack" the creation date, then they are certainly
not "bad", though for the purposes of the uuid function they are -
slightly confusing.
 * BAD_ARGUMENT as in "the FS you requested to id is not a valid
argument to this function", but this err code is usually applied for the
failure of argument precondition checks (non-empty device names, etc.)
 * INVALID_COMMAND as in "this command cannot be applied to that FS"
 * Or even NOT_IMPLEMENTED_YET as in "we still haven't figured how to id
this FS", but this seems to imply that we will eventually do.

By the way, reviewing BAD_ARGUMENT has reminded me that maybe the
function should check whether "uuid" is valid. If not, you don't need to
abort the operation, just the  malloc and consequent sprintf. If
implemented like this in all other FSs, this would be a fast way to
check whether or not a particular FS can have an UUID extracted without
actually having to get the id itself:

       /* ... */
         grub_error (GRUB_ERR_BAD_NUMBER, "No creation date in
filesystem to generate UUID.");
         if (uuid)
           *uuid = NULL;
       }
     else
       {
         if (uuid)
           {
             *uuid = /* ... */

-Habbit

[-- Attachment #2: Esta parte del mensaje está firmada digitalmente --]
[-- Type: application/pgp-signature, Size: 827 bytes --]

Re: [PATCH] iso9660 UUID support by using the creation date/time

[Felix Zielcke]

Am Sonntag, den 31.08.2008, 17:06 +0200 schrieb Javier Martín:

Hello Javier,

> El dom, 31-08-2008 a las 16:27 +0200, Felix Zielcke escribió:
> > 
> > But maybe it would be better to make a new type for this, though I don't
> > have yet an idea how to call it.
> > Suggestions please :)
> What about:
>  * BAD_FS looks the most contextual here, but if the ISO9660 standard
> allows filesystems to "lack" the creation date, then they are certainly
> not "bad", though for the purposes of the uuid function they are -
> slightly confusing.
>  * BAD_ARGUMENT as in "the FS you requested to id is not a valid
> argument to this function", but this err code is usually applied for the
> failure of argument precondition checks (non-empty device names, etc.)
>  * INVALID_COMMAND as in "this command cannot be applied to that FS"
>  * Or even NOT_IMPLEMENTED_YET as in "we still haven't figured how to id
> this FS", but this seems to imply that we will eventually do.

Yep not really easy to find a correct one, but luckly not that important
on grub2's sourcecode the values aren't that often exactly checked as
far as I could see.
This is probable just about `coding style'
Well let's see and wait.

> By the way, reviewing BAD_ARGUMENT has reminded me that maybe the
> function should check whether "uuid" is valid. If not, you don't need to
> abort the operation, just the  malloc and consequent sprintf. If
> implemented like this in all other FSs, this would be a fast way to
> check whether or not a particular FS can have an UUID extracted without
> actually having to get the id itself:

Actually it is currently in a fast way implemented.
iso9660 filesystem currently doestn't have any UUID function at all,
because it doestn't have one strictly speaking.
For normal filesystems if the specs say they have an UUID field they
normally always should have one.
But the problem with iso9660 is that the specs say that the creation
date which I use can be just zeros.
I tend now too like Robert that no UUID for iso9660 + printing an error
might be better then an UUID containing just zeros.


util/grub-probe.c

      if (! fs->uuid)
        grub_util_error ("%s does not support UUIDs", fs->name);

      fs->uuid (dev, &uuid);


For example fs/cpio.c still has no uuid function at all :)

-- 
Felix Zielcke

Re: [PATCH] iso9660 UUID support by using the creation date/time

[Robert Millan]

Some more things I overlooked..

On Sun, Aug 31, 2008 at 04:27:04PM +0200, Felix Zielcke wrote:
> +struct grub_iso9660_date
> +{
> +  grub_uint8_t year[4];
> +  grub_uint8_t month[2];
> +  grub_uint8_t day[2];
> +  grub_uint8_t hour[2];
> +  grub_uint8_t minute[2];
> +  grub_uint8_t second[2];
> +  grub_uint8_t hundredth[2];
> +  grub_uint8_t offset;
> +}__attribute__ ((packed));
    ^

Please add a space here

> @@ -812,8 +826,59 @@ grub_iso9660_label (grub_device_t device
>    return grub_errno;
>  }
>  
> -\f

Please don't remove the ^Ls.
 
-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH] iso9660 UUID support by using the creation date/time

[Felix Zielcke]

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

Am Sonntag, den 31.08.2008, 18:56 +0200 schrieb Robert Millan:
> Some more things I overlooked..

> > +}__attribute__ ((packed));
>     ^
> 
> Please add a space here

Right.

> > @@ -812,8 +826,59 @@ grub_iso9660_label (grub_device_t device
> >    return grub_errno;
> >  }
> >  
> > -\f
> 
> Please don't remove the ^Ls.

It was moved to down, but I forgot an empty line so diff choose to - and + it.
Fixed now in the attached one it's only once shown now without - or + :)

Bean hopefully comments on this too because he did the iso9660 module.
I leaved now the errno to BAD_NUMBER.
 
-- 
Felix Zielcke

[-- Attachment #2: iso9660_uuid.patch.4 --]
[-- Type: text/plain, Size: 3315 bytes --]

2008-08-31  Felix Zielcke  <fzielcke@z-51.de>

	* fs/iso9660.c (grub_iso9660_date): New structure.
	(grub_iso9660_primary_voldesc): Add `grub_iso9660_date' member.
	(grub_iso9660_uuid): New function.

Index: fs/iso9660.c
===================================================================
--- fs/iso9660.c	(Revision 1840)
+++ fs/iso9660.c	(Arbeitskopie)
@@ -67,6 +67,18 @@ struct grub_iso9660_dir
   grub_uint8_t namelen;
 } __attribute__ ((packed));
 
+struct grub_iso9660_date
+{
+  grub_uint8_t year[4];
+  grub_uint8_t month[2];
+  grub_uint8_t day[2];
+  grub_uint8_t hour[2];
+  grub_uint8_t minute[2];
+  grub_uint8_t second[2];
+  grub_uint8_t hundredth[2];
+  grub_uint8_t offset;
+} __attribute__ ((packed));
+
 /* The primary volume descriptor.  Only little endian is used.  */
 struct grub_iso9660_primary_voldesc
 {
@@ -81,6 +93,8 @@ struct grub_iso9660_primary_voldesc
   grub_uint32_t path_table;
   grub_uint8_t unused5[12];
   struct grub_iso9660_dir rootdir;
+  grub_uint8_t unused6[641];
+  struct grub_iso9660_date created;
 } __attribute__ ((packed));
 
 /* A single entry in the path table.  */
@@ -812,6 +826,58 @@ grub_iso9660_label (grub_device_t device
   return grub_errno;
 }
 
+
+static grub_err_t
+grub_iso9660_uuid (grub_device_t device, char **uuid)
+{
+  struct grub_iso9660_data *data;
+  grub_disk_t disk = device->disk;
+
+#ifndef GRUB_UTIL
+  grub_dl_ref (my_mod);
+#endif
+
+  data = grub_iso9660_mount (disk);
+  if (data)
+    {
+      if (! data->voldesc.created.year[0] && ! data->voldesc.created.year[1]
+	  && ! data->voldesc.created.year[2] && ! data->voldesc.created.year[3]
+	  && ! data->voldesc.created.month[0] && ! data->voldesc.created.month[1]
+	  && ! data->voldesc.created.day[0] && ! data->voldesc.created.day[1]
+	  && ! data->voldesc.created.hour[0] && ! data->voldesc.created.hour[1]
+	  && ! data->voldesc.created.minute[0] && ! data->voldesc.created.minute[1]
+	  && ! data->voldesc.created.second[0] && ! data->voldesc.created.second[1]
+	  && ! data->voldesc.created.hundredth[0] && ! data->voldesc.created.hundredth[1])
+	{
+	  grub_error (GRUB_ERR_BAD_NUMBER, "No creation date in filesystem to generate UUID.");
+	  *uuid = NULL;
+	}
+      else 
+	{
+	  *uuid = grub_malloc (sizeof ("YYYY-MM-DD-HH-mm-ss-hh") + sizeof ('\0'));
+	  grub_sprintf (*uuid, "%c%c%c%c-%c%c-%c%c-%c%c-%c%c-%c%c-%c%c",
+			data->voldesc.created.year[0], data->voldesc.created.year[1], 
+			data->voldesc.created.year[2], data->voldesc.created.year[3],
+			data->voldesc.created.month[0], data->voldesc.created.month[1],
+			data->voldesc.created.day[0], data->voldesc.created.day[1],
+			data->voldesc.created.hour[0], data->voldesc.created.hour[1],
+			data->voldesc.created.minute[0], data->voldesc.created.minute[1],
+			data->voldesc.created.second[0], data->voldesc.created.second[1],
+			data->voldesc.created.hundredth[0], data->voldesc.created.hundredth[1]);
+	}
+    }
+  else
+    *uuid = NULL;
+
+#ifndef GRUB_UTIL
+	grub_dl_unref (my_mod);
+#endif
+
+  grub_free (data);
+
+  return grub_errno;
+}
+
 \f
 
 static struct grub_fs grub_iso9660_fs =
@@ -822,6 +888,7 @@ static struct grub_fs grub_iso9660_fs =
     .read = grub_iso9660_read,
     .close = grub_iso9660_close,
     .label = grub_iso9660_label,
+    .uuid = grub_iso9660_uuid,
     .next = 0
   };
 

Re: [PATCH] iso9660 UUID support by using the creation date/time

[Robert Millan]

On Sun, Aug 31, 2008 at 07:23:41PM +0200, Felix Zielcke wrote:
> +	  *uuid = grub_malloc (sizeof ("YYYY-MM-DD-HH-mm-ss-hh")

Lovely :-)

> + sizeof ('\0'));

Is this really needed?  sizeof("foo") implicitly includes the trailing \0.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH] iso9660 UUID support by using the creation date/time

[Felix Zielcke]

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

Am Sonntag, den 31.08.2008, 20:40 +0200 schrieb Robert Millan:
> On Sun, Aug 31, 2008 at 07:23:41PM +0200, Felix Zielcke wrote:
> > +	  *uuid = grub_malloc (sizeof ("YYYY-MM-DD-HH-mm-ss-hh")
> 
> Lovely :-)
> 
> > + sizeof ('\0'));
> 
> Is this really needed?  sizeof("foo") implicitly includes the trailing \0.

Args, I'm blind :(
I copied the function from fs/ntfs.c and there it says `grub_malloc 16 + sizeof (\0)'
With "" it's useless so .5 attached so it doestn't get lost :)

-- 
Felix Zielcke

[-- Attachment #2: iso9660_uuid.patch.5 --]
[-- Type: text/plain, Size: 3299 bytes --]

2008-08-31  Felix Zielcke  <fzielcke@z-51.de>

	* fs/iso9660.c (grub_iso9660_date): New structure.
	(grub_iso9660_primary_voldesc): Add `grub_iso9660_date' member.
	(grub_iso9660_uuid): New function.

Index: fs/iso9660.c
===================================================================
--- fs/iso9660.c	(Revision 1840)
+++ fs/iso9660.c	(Arbeitskopie)
@@ -67,6 +67,18 @@ struct grub_iso9660_dir
   grub_uint8_t namelen;
 } __attribute__ ((packed));
 
+struct grub_iso9660_date
+{
+  grub_uint8_t year[4];
+  grub_uint8_t month[2];
+  grub_uint8_t day[2];
+  grub_uint8_t hour[2];
+  grub_uint8_t minute[2];
+  grub_uint8_t second[2];
+  grub_uint8_t hundredth[2];
+  grub_uint8_t offset;
+} __attribute__ ((packed));
+
 /* The primary volume descriptor.  Only little endian is used.  */
 struct grub_iso9660_primary_voldesc
 {
@@ -81,6 +93,8 @@ struct grub_iso9660_primary_voldesc
   grub_uint32_t path_table;
   grub_uint8_t unused5[12];
   struct grub_iso9660_dir rootdir;
+  grub_uint8_t unused6[641];
+  struct grub_iso9660_date created;
 } __attribute__ ((packed));
 
 /* A single entry in the path table.  */
@@ -812,6 +826,58 @@ grub_iso9660_label (grub_device_t device
   return grub_errno;
 }
 
+
+static grub_err_t
+grub_iso9660_uuid (grub_device_t device, char **uuid)
+{
+  struct grub_iso9660_data *data;
+  grub_disk_t disk = device->disk;
+
+#ifndef GRUB_UTIL
+  grub_dl_ref (my_mod);
+#endif
+
+  data = grub_iso9660_mount (disk);
+  if (data)
+    {
+      if (! data->voldesc.created.year[0] && ! data->voldesc.created.year[1]
+	  && ! data->voldesc.created.year[2] && ! data->voldesc.created.year[3]
+	  && ! data->voldesc.created.month[0] && ! data->voldesc.created.month[1]
+	  && ! data->voldesc.created.day[0] && ! data->voldesc.created.day[1]
+	  && ! data->voldesc.created.hour[0] && ! data->voldesc.created.hour[1]
+	  && ! data->voldesc.created.minute[0] && ! data->voldesc.created.minute[1]
+	  && ! data->voldesc.created.second[0] && ! data->voldesc.created.second[1]
+	  && ! data->voldesc.created.hundredth[0] && ! data->voldesc.created.hundredth[1])
+	{
+	  grub_error (GRUB_ERR_BAD_NUMBER, "No creation date in filesystem to generate UUID.");
+	  *uuid = NULL;
+	}
+      else 
+	{
+	  *uuid = grub_malloc (sizeof ("YYYY-MM-DD-HH-mm-ss-hh"));
+	  grub_sprintf (*uuid, "%c%c%c%c-%c%c-%c%c-%c%c-%c%c-%c%c-%c%c",
+			data->voldesc.created.year[0], data->voldesc.created.year[1], 
+			data->voldesc.created.year[2], data->voldesc.created.year[3],
+			data->voldesc.created.month[0], data->voldesc.created.month[1],
+			data->voldesc.created.day[0], data->voldesc.created.day[1],
+			data->voldesc.created.hour[0], data->voldesc.created.hour[1],
+			data->voldesc.created.minute[0], data->voldesc.created.minute[1],
+			data->voldesc.created.second[0], data->voldesc.created.second[1],
+			data->voldesc.created.hundredth[0], data->voldesc.created.hundredth[1]);
+	}
+    }
+  else
+    *uuid = NULL;
+
+#ifndef GRUB_UTIL
+	grub_dl_unref (my_mod);
+#endif
+
+  grub_free (data);
+
+  return grub_errno;
+}
+
 \f
 
 static struct grub_fs grub_iso9660_fs =
@@ -822,6 +888,7 @@ static struct grub_fs grub_iso9660_fs =
     .read = grub_iso9660_read,
     .close = grub_iso9660_close,
     .label = grub_iso9660_label,
+    .uuid = grub_iso9660_uuid,
     .next = 0
   };
 

Re: [PATCH] iso9660 UUID support by using the creation date/time

[Felix Zielcke]

Am Sonntag, den 31.08.2008, 20:55 +0200 schrieb Felix Zielcke:

> Args, I'm blind :(
> I copied the function from fs/ntfs.c and there it says `grub_malloc 16 + sizeof (\0)'
> With "" it's useless so .5 attached so it doestn't get lost :)

Bean did only the joliet support and he already said it looks okay.
But Marco I'd like to have your comment because you initially coded the
iso9660 support :)

Re: [PATCH] iso9660 UUID support by using the creation date/time

[Robert Millan]

On Fri, Sep 05, 2008 at 07:02:57PM +0200, Felix Zielcke wrote:
> Am Sonntag, den 31.08.2008, 20:55 +0200 schrieb Felix Zielcke:
> 
> > Args, I'm blind :(
> > I copied the function from fs/ntfs.c and there it says `grub_malloc 16 + sizeof (\0)'
> > With "" it's useless so .5 attached so it doestn't get lost :)
> 
> Bean did only the joliet support and he already said it looks okay.
> But Marco I'd like to have your comment because you initially coded the
> iso9660 support :)

The patch is completely non-intrusive, and nobody had objections or other
comments for a while.  I think it's fine if you check it in.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH] iso9660 UUID support by using the creation date/time

[Felix Zielcke]

Am Samstag, den 06.09.2008, 13:11 +0200 schrieb Robert Millan:
> On Fri, Sep 05, 2008 at 07:02:57PM +0200, Felix Zielcke wrote:
> > Bean did only the joliet support and he already said it looks okay.
> > But Marco I'd like to have your comment because you initially coded the
> > iso9660 support :)
> 
> The patch is completely non-intrusive, and nobody had objections or other
> comments for a while.  I think it's fine if you check it in.

Thanks Robert commited :)