All patches committed
Thanks
On 11/11/2010 09:56 AM, Giuseppe Caizzone wrote:
> 2010/11/6 Vladimir 'φ-coder/phcoder' Serbinenko <phco...@gmail.com>:
>
>> On 11/02/2010 02:10 PM, Giuseppe Caizzone wrote:
>>
>>> Hello,
>>>
>>> I've made 4 small patches that improve the UDF filesystem support in
>>> grub. With these changes, I've been able to read any regular file in
>>> UDF filesystems created by both Linux and Windows on both optical
>>> storage and USB sticks.
>>> [...]
>>> 1/4) Support for large (> 2GiB) files
>>> [...]
>>> The patch just changes an int and a grub_uint32_t to be grub_off_t.
>>>
>>>
>>>
>> Good patch. Can you write the ChangeLog entry for it?
>>
> OK - I've never written one, so I tried to mimic grub's.
>
> Support reading files larger than 2 GiB.
>
> * grub-core/fs/udf.c (grub_udf_iterate_dir): change type of variable
> "offset" from grub_uint32_t to grub_off_t.
> (grub_udf_read_file): change type of parameter "pos" from int to
> grub_off_t.
>
>
>>> 2/4) Support for deleted files
>>> [...]
>>> The patch just skips directory entries with the "deleted" bit set, in
>>> grub_udf_iterate_dir().
>>>
>>>
>>>
>> This one is good too. Changelog entry?
>>
> Properly handle deleted files.
>
> * grub-core/fs/udf.c (grub_udf_iterate_dir): Skip directory entries
> whose "characteristics" field has the bit GRUB_UDF_FID_CHAR_DELETED
> set.
>
>
>>> 3/4) Support for a generic block size
>>> [...]
>>> The patch repeats the superblock search in grub_udf_mount() for 512,
>>> 1024, 2048 and 4096 block sizes, stopping if it finds a valid one AND
>>> the sector offset recorded in that superblock matches the detected
>>> superblock offset. So for instance it won't misdetect a superblock
>>> located at sector 256 with blocksize 2048 for a superblock located at
>>> sector 512 with blocksize 1024.
>>> The log2(blocksize/512) is then stored in a new field called "lbshift"
>>> in struct grub_udf_data, which gets used instead of the previous
>>> constant GRUB_UDF_LOG2_BLKSZ, while the constant GRUB_UDF_BLKSZ gets
>>> replaced with the lvd.bsize field already present in struct
>>> grub_udf_data.
>>>
>>>
>>>
>> + lbshift = 0;
>> + block = 0;
>> + while (lbshift <= 3)
>> + {
>> For loop is way more appropriate than a while loop
>>
> Changed it into a for loop.
>
>
>> + while (lbshift <= 3)
>> + {
>> + sblklist = sblocklist;
>> + while (*sblklist)
>> + {
>> Same here.
>>
> Changed that too.
>
> I also made another change since the previous version of the patch: I
> moved the VRS check after the AVDP search, because it needs to know
> the logical block size. I originally thought that it wasn't necessary,
> because the VRS is made up of records with a fixed size of 2048 bytes,
> but the catch is that the standard says that each record must start on
> a new sector, so if block size is > 2048, one has to skip more bytes
> than 2048 to get to the next record. Tested it with mkudffs because I
> have no medium with an actual block size of 4096.
>
>
>> ChangeLog entry?
>>
> Add generic logical block size support.
>
> * grub-core/fs/udf.c (GRUB_UDF_LOG2_BLKSIZE): Removed macro.
> (GRUB_UDF_BLKSZ): Removed macro.
> (struct grub_udf_data): New field "lbshift" to hold the logical block
> size of the file system in log2 format.
> (grub_udf_read_icb): Replace usage of macro GRUB_UDF_LOG2_BLKSZ with
> field lbshift from struct grub_udf_data.
> (grub_udf_read_file): Likewise.
> (grub_udf_read_block): Replace usage of macro GRUB_UDF_BLKSZ with
> field "lvd.bsize" from struct grub_udf_data.
> Replace division with right shift.
> (sblocklist): Change type to unsigned.
> (grub_udf_mount): Change type of "sblklist" to unsigned.
> New variable "vblock" to be used during VRS recognition.
> New variable "lbshift" to hold the detected logical block size.
> Move AVDP search before VRS recognition, because the latter requires
> knowledge of the logical block size, which is detected during the
> former.
> Detect and validate logical block size during AVDP search, adding
> support for block sizes 512, 1024 and 4096.
> Make VRS recognition independent of block size.
> Replace usages of macro GRUB_UDF_LOG2_BLKSZ with variable lbshift.
>
>
>>> 4/4) Support for allocation descriptor extensions
>>> [...]
>>> The patch checks, in grub_udf_read_block(), whether the "allocation
>>> descriptor type", previously ignored, is 3, and in this case, it loads
>>> the referenced block, checks that it's a valid allocation extension
>>> descriptor, and sets the "ad" pointer (and the "len" limit) to
>>> continue the iteration from the allocation descriptors contained in
>>> that block.
>>>
>>> This last one is the only patch which actually contains a proper new
>>> block of code, and it allocates a big structure on the stack (it's a
>>> sector, so it's up to 4K big): I don't know if this is OK, or if
>>> perhaps I should use grub_malloc() instead. Also, the patch depends on
>>> the previous "generic block size" patch.
>>>
>>>
>>>
>> + char buf[U32 (node->data->lvd.bsize)];
>> Will segfault if bsize is too big. You need to allocate on heap
>>
> Done.
>
>
>> - else
>> + else if (U16 (node->fe.tag.tag_ident == GRUB_UDF_TAG_IDENT_EFE))
>> {
>> Parenthesis are wrong.
>>
> While I was at it, I changed the 'if-else if' to a switch.
>
>
>> + grub_disk_addr_t sec = grub_udf_get_block(node->data,
>> + node->part_ref,
>> + ad->position);
>> ad->position isn't byte-swapped.
>>
> grub_udf_get_block() byte-swaps it itself, so I must not byte-swap it before.
>
>
>> ChangeLog entry?
>>
> Add support for allocation extent descriptors, needed on fragmented
> volumes.
>
> * grub-core/fs/udf.c (grub_udf_aed): New struct.
> (grub_udf_read_block): Change type of variable "len" to grub_ssize_t.
> Add sanity check for file entry type.
> Replace divisions with right shifts.
> Check the upper bits of the allocation descriptor's length and honour
> them by loading an allocation extent descriptor if they indicate so.
> Change all failure return paths to free the allocation extent
> descriptor if it was allocated.
>
> I'm attaching the full patch set (including the unchanged ones for
> convenience), and also the change log entries in attachment form in
> case gmail tampers with whitespace.
>
> Thank you,
>
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
> -- Regards Vladimir 'φ-coder/phcoder' Serbinenko
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
