public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Stephen Warren <swarren@wwwdotorg.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/2] fs/fat/fatwrite: Local variable as buffer to store dir_slot entries
Date: Thu, 14 Jul 2016 15:00:41 -0600	[thread overview]
Message-ID: <5787FD79.5020205@wwwdotorg.org> (raw)
In-Reply-To: <1468400470-28485-1-git-send-email-tfchee@altera.com>

On 07/13/2016 03:01 AM, Tien Fong Chee wrote:
> fill_dir_slot use get_contents_vfatname_block as a temporary buffer for
> constructing a list of dir_slot entries. To save the memory and providing
> correct type of memory for above usage, a local buffer with accurate size
> declaration is introduced.
>
> The local array size 640 is used because for long file name entry,
> each entry use 32 bytes, one entry can store up to 13 characters.
> The maximum number of entry possible is 20. So, total size is
> 32*20=640bytes.

I'm fairly sure this series is correct[1], although the FAT write code 
is a little hard to follow.

[1] except in the face of disk read errors or corrupted data while 
parsing long filename entries, but the code already looks broken in the 
case of disk errors, and the corrupted data case probably isn't much 
worse now.

> diff --git a/include/fat.h b/include/fat.h

> +/* Maximum number of entry for long file name according to spec */
> +#define MAX_LFN_SLOT	20
> +
>
>   /* Filesystem identifiers */
>   #define FAT12_SIGN	"FAT12   "

Why 2 blank lines there?

Also, I'd suggest simply deleting get_contents_vfatname_block and 
renaming all references to it to use get_dentfromdir_block instead. That 
way, anyone reading the code in the future won't assume the two "block" 
variables refer to different memory, when in fact they share storage. 
Related, there's little point keeping the now-redundant memcpy() call at 
the end of get_long_file_name():

> 		memcpy(get_dentfromdir_block, get_contents_vfatname_block,
> 			mydata->clust_size * mydata->sect_size);

  parent reply	other threads:[~2016-07-14 21:00 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-13  9:01 [U-Boot] [PATCH 1/2] fs/fat/fatwrite: Local variable as buffer to store dir_slot entries Tien Fong Chee
2016-07-13  9:01 ` [U-Boot] [PATCH 2/2] fs/fat/fat: Optimizes memory size with single global variable instead of 3 Tien Fong Chee
2016-07-13 10:56   ` Benoît Thébaudeau
2016-07-14 10:48     ` Tien Fong Chee
2016-07-14 23:37       ` Benoît Thébaudeau
2016-07-19  3:53         ` Tien Fong Chee
2016-07-19 16:25           ` Stephen Warren
2016-07-14 21:00 ` Stephen Warren [this message]
2016-07-19  3:24   ` [U-Boot] [PATCH 1/2] fs/fat/fatwrite: Local variable as buffer to store dir_slot entries Tien Fong Chee
  -- strict thread matches above, loose matches on Subject: below --
2016-07-14 10:01 Tien Fong Chee
2016-07-28  6:08 Tien Fong Chee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5787FD79.5020205@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox