[U-Boot] [PATCH v2 3/8] fat/fs: convert to directory iterators

Rob Clark
2017-09-06T02:18:30Z

On Tue, Sep 5, 2017 at 4:56 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Rob,
>
> On 3 September 2017 at 23:08, Łukasz Majewski <lukma@denx.de> wrote:
>> On 09/02/2017 06:37 PM, Rob Clark wrote:
>>>
>>> And drop a whole lot of ugly code!
>
> :-)
>
>>
>>
>> +1
>>
>> Reviewed-by: Łukasz Majewski <lukma@denx.de>
>>
>>
>>
>>>
>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>> ---
>>> fs/fat/fat.c | 723
>>> ++++++----------------------------------------------------
>>> include/fat.h | 6 -
>>> 2 files changed, 75 insertions(+), 654 deletions(-)
>>>
>>> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
>>> index c72d6ca931..3193290434 100644
>>> --- a/fs/fat/fat.c
>>> +++ b/fs/fat/fat.c
>>> @@ -119,22 +119,6 @@ int fat_register_device(struct blk_desc *dev_desc,
>>> int part_no)
>>> }
>>> /*
>>> - * Get the first occurence of a directory delimiter ('/' or '\') in a
>>> string.
>>> - * Return index into string if found, -1 otherwise.
>>> - */
>>> -static int dirdelim(char *str)
>>> -{
>>> - char *start = str;
>>> -
>>> - while (*str != '\0') {
>>> - if (ISDIRDELIM(*str))
>>> - return str - start;
>>> - str++;
>>> - }
>>> - return -1;
>>> -}
>>> -
>>> -/*
>>> * Extract zero terminated short name from a directory entry.
>>> */
>>> static void get_name(dir_entry *dirent, char *s_name)
>>> @@ -468,95 +452,6 @@ static int slot2str(dir_slot *slotptr, char *l_name,
>>> int *idx)
>>> return 0;
>>> }
>>> -/*
>>> - * Extract the full long filename starting at 'retdent' (which is really
>>> - * a slot) into 'l_name'. If successful also copy the real directory
>>> entry
>>> - * into 'retdent'
>>> - * Return 0 on success, -1 otherwise.
>>> - */
>>> -static int
>>> -get_vfatname(fsdata *mydata, int curclust, __u8 *cluster,
>>> - dir_entry *retdent, char *l_name)
>>> -{
>>> - dir_entry *realdent;
>>> - dir_slot *slotptr = (dir_slot *)retdent;
>>> - __u8 *buflimit = cluster + mydata->sect_size * ((curclust == 0) ?
>>> - PREFETCH_BLOCKS :
>>> -
>>> mydata->clust_size);
>>> - __u8 counter = (slotptr->id & ~LAST_LONG_ENTRY_MASK) & 0xff;
>>> - int idx = 0;
>>> -
>>> - if (counter > VFAT_MAXSEQ) {
>>> - debug("Error: VFAT name is too long\n");
>>> - return -1;
>>> - }
>>> -
>>> - while ((__u8 *)slotptr < buflimit) {
>>> - if (counter == 0)
>>> - break;
>>> - if (((slotptr->id & ~LAST_LONG_ENTRY_MASK) & 0xff) !=
>>> counter)
>>> - return -1;
>>> - slotptr++;
>>> - counter--;
>>> - }
>>> -
>>> - if ((__u8 *)slotptr >= buflimit) {
>>> - dir_slot *slotptr2;
>>> -
>>> - if (curclust == 0)
>>> - return -1;
>>> - curclust = get_fatent(mydata, curclust);
>>> - if (CHECK_CLUST(curclust, mydata->fatsize)) {
>>> - debug("curclust: 0x%x\n", curclust);
>>> - printf("Invalid FAT entry\n");
>>> - return -1;
>>> - }
>>> -
>>> - if (get_cluster(mydata, curclust,
>>> get_contents_vfatname_block,
>>> - mydata->clust_size * mydata->sect_size) !=
>>> 0) {
>>> - debug("Error: reading directory block\n");
>>> - return -1;
>>> - }
>>> -
>>> - slotptr2 = (dir_slot *)get_contents_vfatname_block;
>>> - while (counter > 0) {
>>> - if (((slotptr2->id & ~LAST_LONG_ENTRY_MASK)
>>> - & 0xff) != counter)
>>> - return -1;
>>> - slotptr2++;
>>> - counter--;
>>> - }
>>> -
>>> - /* Save the real directory entry */
>>> - realdent = (dir_entry *)slotptr2;
>>> - while ((__u8 *)slotptr2 > get_contents_vfatname_block) {
>>> - slotptr2--;
>>> - slot2str(slotptr2, l_name, &idx);
>>> - }
>>> - } else {
>>> - /* Save the real directory entry */
>>> - realdent = (dir_entry *)slotptr;
>>> - }
>>> -
>>> - do {
>>> - slotptr--;
>>> - if (slot2str(slotptr, l_name, &idx))
>>> - break;
>>> - } while (!(slotptr->id & LAST_LONG_ENTRY_MASK));
>>> -
>>> - l_name[idx] = '\0';
>>> - if (*l_name == DELETED_FLAG)
>>> - *l_name = '\0';
>>> - else if (*l_name == aRING)
>>> - *l_name = DELETED_FLAG;
>>> - downcase(l_name);
>>> -
>>> - /* Return the real directory entry */
>>> - memcpy(retdent, realdent, sizeof(dir_entry));
>>> -
>>> - return 0;
>>> -}
>>> -
>>> /* Calculate short name checksum */
>>> static __u8 mkcksum(const char name[8], const char ext[3])
>>> {
>>> @@ -572,170 +467,11 @@ static __u8 mkcksum(const char name[8], const char
>>> ext[3])
>>> return ret;
>>> }
>>> -/*
>>> - * Get the directory entry associated with 'filename' from the directory
>>> - * starting at 'startsect'
>>> - */
>>> +// These should probably DIAF..
>
> Can you use /* ?
>
> Perhaps a TODO here would help - are you suggesting using malloc()?

I'll convert to /* TODO .. */

I didn't mean to use malloc, but iirc at least some of this goes away
when fat_write.c gets converted to use directory iterators.. I think
these are only used directly or indirectly by do_fat_write()..

I may have to tackle fat_write.c in the near future for UEFI SCT test
suite (un)fortunately.. might be after next merge-window but I think
that TODO can be tackled in the near future.

BR,
-R

> Did this patch go through patman/checkpatch?
>
> Regards,
> Simon