Hi Sergei,
Thank you for the second version of the patch. It looks much better. But
it is easier to answer on the patch in the e-mail body. Anyway, you
should send the next (and, maybe, final) version of the patch in the
e-mail body.
>> This patch adds support for HFSX 'HasFolderCount' flag and a corresponding 'folderCount' field in folder records. (For reference see HFS_FOLDERCOUNT and kHFSHasFolderCountBit/kHFSHasFolderCountMask in Apple's source code.)
Please, use 80 symbol on the line rule and for patch's comment too.
>> diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
>> index 08846425b..e1911fc 100644
>> --- a/fs/hfsplus/hfsplus_fs.h
>> +++ b/fs/hfsplus/hfsplus_fs.h
>> @@ -242,6 +242,8 @@ struct hfsplus_inode_info {
>> */
>> sector_t fs_blocks;
>> u8 userflags; /* BSD user file flags */
>> + u32 folder_count; /* Subfolder count if
>> + * HFSPLUS_HAS_FOLDER_COUNT is set */
I suggest to change comment here: "Subfolders count (HFSX only)".
And it makes sense to add more detailed comment for
HFSPLUS_HAS_FOLDER_COUNT flag.
>> diff --git a/fs/hfsplus/hfsplus_raw.h b/fs/hfsplus/hfsplus_raw.h
>> index 8ffb3a8..61aeea8 100644
>> --- a/fs/hfsplus/hfsplus_raw.h
>> +++ b/fs/hfsplus/hfsplus_raw.h
>> @@ -261,7 +261,10 @@ struct hfsplus_cat_folder {
>> struct DInfo user_info;
>> struct DXInfo finder_info;
>> __be32 text_encoding;
>> - u32 reserved;
>> +
>> + /* Number of subfolders when HFSPLUS_HAS_FOLDER_COUNT flag is set.
>> + * Reserved otherwise. */
>> + __be32 folder_count;
What about shorter name as "subfolders" for both fields in struct
hfsplus_inode_info and in struct hfsplus_cat_folder? I suppose it will
be more informative and correct.
I suggest to change comment here: "Subfolders count (HFSX only).
Reserved otherwise.". And it makes sense to add more detailed comment
for HFSPLUS_HAS_FOLDER_COUNT flag.
>> diff --git a/fs/hfsplus/catalog.c b/fs/hfsplus/catalog.c
>> index 968ce41..04228d6 100644
>> --- a/fs/hfsplus/catalog.c
>> +++ b/fs/hfsplus/catalog.c
>> @@ -103,6 +103,8 @@ static int hfsplus_cat_build_record(hfsplus_cat_entry *entry,
>> folder = &entry->folder;
>> memset(folder, 0, sizeof(*folder));
>> folder->type = cpu_to_be16(HFSPLUS_FOLDER);
>>+ if (test_bit(HFSPLUS_SB_HFSX, &sbi->flags))
>>+ folder->flags |= cpu_to_be16(HFSPLUS_HAS_FOLDER_COUNT);
Are you sure that on every HFSX volume folder records in CatalogFile
have HFSPLUS_HAS_FOLDER_COUNT flag as set? Maybe it can depend from some
flag or hints in superblock? I simply want to be sure that you tested
all key situations.
Thanks,
Vyacheslav Dubeyko.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
