public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Aaron Williams <Aaron.Williams@cavium.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH V2] fs: fat: Fix mkcksum() function parameters
Date: Tue, 08 Jan 2013 17:54:41 -0800	[thread overview]
Message-ID: <50ECCDE1.2020707@cavium.com> (raw)
In-Reply-To: <1349803222-15917-1-git-send-email-marex@denx.de>

Hi Marek,

This patch is broken. It breaks detecting duplicate filenames. The 
problem is that you are using sizeof(name) and sizeof(ext). This does 
not work since this just reports the size of the pointer in gcc 4.7, 
which according to our compiler guy is the correct behavior. Instead of 
using sizeof, just use 8 and 3 respectively. Otherwise it is doing a 
checksum of 4 and 4 characters.

-Aaron

On 10/09/2012 10:20 AM, Marek Vasut wrote:
> The mkcksum() function now takes one parameter, the pointer to
> 11-byte wide character array, which it then operates on.
>
> Currently, the function is wrongly passed (dir_entry)->name, which
> is only 8-byte wide character array. Though by further inspecting
> the dir_entry structure, it can be noticed that the name[8] entry
> is immediatelly followed by ext[3] entry. Thus, name[8] and ext[3]
> in the dir_entry structure actually work as this 11-byte wide array
> since they're placed right next to each other by current compiler
> behavior.
>
> Depending on this is obviously wrong, thus fix this by correctly
> passing both (dir_entry)->name and (dir_entry)->ext to the mkcksum()
> function and adjust the function appropriately.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Tom Rini <trini@ti.com>
> ---
>   fs/fat/fat.c       |   20 ++++++++++++--------
>   fs/fat/fat_write.c |    2 +-
>   2 files changed, 13 insertions(+), 9 deletions(-)
>
> V2: Fix fat write by adding braces
>
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index 41ae15e..8870292 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -567,15 +567,16 @@ get_vfatname(fsdata *mydata, int curclust, __u8 *cluster,
>   }
>   
>   /* Calculate short name checksum */
> -static __u8 mkcksum(const char *str)
> +static __u8 mkcksum(const char name[8], const char ext[3])
>   {
>   	int i;
>   
>   	__u8 ret = 0;
>   
> -	for (i = 0; i < 11; i++) {
> -		ret = (((ret & 1) << 7) | ((ret & 0xfe) >> 1)) + str[i];
> -	}
> +	for (i = 0; i < sizeof(name); i++)
> +		ret = (((ret & 1) << 7) | ((ret & 0xfe) >> 1)) + name[i];
> +	for (i = 0; i < sizeof(ext); i++)
> +		ret = (((ret & 1) << 7) | ((ret & 0xfe) >> 1)) + ext[i];
>   
>   	return ret;
>   }
> @@ -678,7 +679,8 @@ static dir_entry *get_dentfromdir(fsdata *mydata, int startsect,
>   				return NULL;
>   			}
>   #ifdef CONFIG_SUPPORT_VFAT
> -			if (dols && mkcksum(dentptr->name) == prevcksum) {
> +			__u8 csum = mkcksum(dentptr->name, dentptr->ext);
> +			if (dols && csum == prevcksum) {
>   				prevcksum = 0xffff;
>   				dentptr++;
>   				continue;
> @@ -946,13 +948,16 @@ do_fat_read_at(const char *filename, unsigned long pos, void *buffer,
>   
>   		for (i = 0; i < DIRENTSPERBLOCK; i++) {
>   			char s_name[14], l_name[VFAT_MAXLEN_BYTES];
> +			__u8 csum;
>   
>   			l_name[0] = '\0';
>   			if (dentptr->name[0] == DELETED_FLAG) {
>   				dentptr++;
>   				continue;
>   			}
> -			if ((dentptr->attr & ATTR_VOLUME)) {
> +
> +			csum = mkcksum(dentptr->name, dentptr->ext);
> +			if (dentptr->attr & ATTR_VOLUME) {
>   #ifdef CONFIG_SUPPORT_VFAT
>   				if ((dentptr->attr & ATTR_VFAT) == ATTR_VFAT &&
>   				    (dentptr->name[0] & LAST_LONG_ENTRY_MASK)) {
> @@ -1015,8 +1020,7 @@ do_fat_read_at(const char *filename, unsigned long pos, void *buffer,
>   				goto exit;
>   			}
>   #ifdef CONFIG_SUPPORT_VFAT
> -			else if (dols == LS_ROOT &&
> -				 mkcksum(dentptr->name) == prevcksum) {
> +			else if (dols == LS_ROOT && csum == prevcksum) {
>   				prevcksum = 0xffff;
>   				dentptr++;
>   				continue;
> diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
> index 5829adf..4a1bda0 100644
> --- a/fs/fat/fat_write.c
> +++ b/fs/fat/fat_write.c
> @@ -335,7 +335,7 @@ fill_dir_slot(fsdata *mydata, dir_entry **dentptr, const char *l_name)
>   
>   	/* Get short file name and checksum value */
>   	strncpy(s_name, (*dentptr)->name, 16);
> -	checksum = mkcksum(s_name);
> +	checksum = mkcksum((*dentptr)->name, (*dentptr)->ext);
>   
>   	do {
>   		memset(slotptr, 0x00, sizeof(dir_slot));


-- 
Aaron Williams
Software Engineer
Cavium, Inc.
(408) 943-7198  (510) 789-8988 (cell)

  parent reply	other threads:[~2013-01-09  1:54 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-30  0:49 [U-Boot] [PATCH] fs: fat: Fix mkcksum() function parameters Marek Vasut
2012-10-09 16:45 ` Tom Rini
2012-10-09 17:20 ` [U-Boot] [PATCH V2] " Marek Vasut
2012-10-17 15:03   ` [U-Boot] [U-Boot, " Tom Rini
2013-01-09  1:54   ` Aaron Williams [this message]
2013-01-09  9:14     ` [U-Boot] [PATCH " Marek Vasut

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=50ECCDE1.2020707@cavium.com \
    --to=aaron.williams@cavium.com \
    --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