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