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

[Rob Clark]

On Tue, Sep 5, 2017 at 4:56 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Rob,
>
> On 3 September 2017 at 23:08, Łukasz Majewski <lukma at 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 at denx.de>
>>
>>
>>
>>>
>>> Signed-off-by: Rob Clark <robdclark at 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