Re: How to go on with ISO 9660 large file support ?

["Thomas Schmitt" <scdbackup%gmx.net@localhost>]

Hi,

Christos Zoulas:
> I'd say it is not critical that those work if we have to run through
> hoops to achieve compatibility. I.e. clean code would be my priority.

The fully API compatible code is slightly cleaner than the
only ABI compatible code, which saves sizeof(void *) with
every vnode/iso_node.

The main advantage of the API-compatible-wasteful variant
is that kmem_free() is a bit less frightening. (Actually
fully non-scary would be malloc(9)/free(9).)

        if (ip->iso_sections != NULL)
                kmem_free(ip->iso_sections,
                          CD9660_FSECT_FROM_INO(ip->i_number)
                          * sizeof(struct iso_file_section));
        ip->iso_sections = NULL;

versus

        if (CD9660_FSECT_FROM_INO(ip->i_number) > 1)
                kmem_free(ip->iso_fsect.many,
                          CD9660_FSECT_FROM_INO(ip->i_number)
                          * sizeof(struct iso_file_section));
        iso_set_ino_nfsect(&ip->i_number, 1);


Another advantage of API-compatible-wasteful is that there will
be no new union defined in cd9660_node.h

--------------------------------------------------------------
API compatible struct iso_node (freestyle diff):

+ struct iso_file_section {
+       unsigned long isofsc_size;      /* byte count */
+       unsigned long isofsc_start;     /* block address */
+ };
+
  struct iso_node {
        ...
-       unsigned long iso_extent;       /* extent of file */
-       unsigned long i_size;
-       unsigned long iso_start;                /* actual start of data of file 
(may be different */
+                                       /* These three numbers represent the
+                                        * first extent and file section.
+                                        * If CD9660_FSECT_FROM_INO(.i_number)
+                                        * is larger than 1, then .iso_sections
+                                        * is valid.
+                                        */
+       unsigned long iso_extent;       /* will not be used by cd9660 */
+       unsigned long i_size;           /* byte count */
+       unsigned long iso_start;        /* block address */
        ...
+       struct iso_file_section *iso_sections;  /* Holds info about all file
+                                                * sections if
+                                                * CD9660_FSECT_FROM_INO(
+                                                *                   .i_number)
+                                                * is larger than 1.
+                                                */
  };

--------------------------------------------------------------
ABI-only compatible:

+ struct iso_file_section {
+       unsigned long isofsc_extent;    /* will not be used by cd9660 */
+       unsigned long isofsc_size;      /* byte count */
+       unsigned long isofsc_start;     /* block address */
+ };
+ union iso_file_data {
+       struct iso_file_section single; /* used if only one extent exists:
+                                        * CD9660_FSECT_FROM_INO(.i_number)==1
+                                        */
+       struct iso_file_section *many;  /* allocated with enough array elements
+                                        * for CD9660_FSECT_FROM_INO(.i_number)
+                                        * if this is larger than 1.
+                                        */
+ };
+
  struct iso_node {
        ...
-       unsigned long iso_extent;       /* extent of file */
-       unsigned long i_size;
-       unsigned long iso_start;                /* actual start of data of file 
(may be different */
+       union iso_file_data iso_fsect;  /* File sections. Their number is given
+                                        * by CD9660_FSECT_FROM_INO(.i_number)
+                                        */
        ...
  };

--------------------------------------------------------------

Have a nice day :)

Thomas