Dear Wolfgang Denk, Thanks for your comments, please see mine below.
> -----Oorspronkelijk bericht----- > Van: Wolfgang Denk [mailto:[email protected]] > Verzonden: donderdag 16 mei 2013 7:55 > Aan: Ruud Commandeur > CC: [email protected]; Tom Rini; Mats K?rrman > Onderwerp: Re: [U-Boot] [PATCH] mmc and fat bug fixes > > Dear Ruud Commandeur, > > In message > <[email protected] > elux.lokaal> you wrote: > > This patch fixes a number of mmc and fat-related bugs: > > > > > Added a check for blkcnt > 0 in mmc_write_blocks > (drivers/mmc.c) to preve= > > nt a hangup for further mmc commands. > > > > > Solved a checksum issue in fs/fat/fat.c. The mkcksum has > const char argum= > > ents with a size specifier, like "const char name[8]". In > the function, it = > > is assumed that sizeof(name) will have the value 8, but > this is not the cas= > > e (at least not for the Sourcery CodeBench compiler and > probably not accord= > > ing to ANSI C). This causes "long filename checksum errors" > for each fat fi= > > le listed or written. > > Please explain. Under which exact conditions would sizeof(name) not > be 8, and where is such assumption supported in ANSI C? > > > I am tempted to NAK the FAT changes, as they make the code much harder > to read and to maintain. > > Using this simple test program: > > ----- snip ---- > #include <stdio.h> > > int main(void) > { > const char name[8]; > const char ext[3]; > > printf("sizeof(name)=%d, expected 8\n", sizeof(name)); > printf("sizeof(ext) =%d, expected 3\n", sizeof(name)); > > return 0; > } > ----- snip ---- > > I get the expected values on all systems and with all compilers I > tested. For which exact configuration do you get different results? > The difference is, that in the original code, the sizeof( ) is used on function arguments. And in that case the result will be the size of the pointer, regardless the addition of size indicators of the arguments. ----- snip ---- #include <stdio.h> const char g_name[8]; const char g_ext[3]; static void mkcksum(const char name[8], const char ext[3]) { printf("sizeof(name)=%d\n", sizeof(name)); printf("sizeof(ext) =%d\n", sizeof(ext)); } int main(void) { printf("sizeof(g_name)=%d\n", sizeof(g_name)); printf("sizeof(g_ext) =%d\n", sizeof(g_ext)); mkcksum(g_name, g_ext); return 0; } ----- snip ---- In this case, the results will be: sizeof(g_name)=8 sizeof(g_ext) =3 sizeof(name)=4 /* will be machine dependant */ sizeof(ext) =4 /* will be machine dependant */ > > > Made some changes to fs/fat/fat_write.c. Fixed testing > fat_val for 0xffff= > > /0xfff8 and 0xfffffff/0xffffff8 by adding the corresponding > fatsize in the = > > test (as read in earlier posts) and some changes in debug output. > > Please restrict your line length in commit messages to some 70 > characters or so. > I will try... > > > Signed-off-by: Ruud Commandeur <[email protected]> > > Cc: Tom Rini <[email protected]> > > Cc: Beno=EEt Th=E9baudeau <[email protected]> > > Cc: Mats Karrman <[email protected]> > > Please split into three separate patches, one for MMX, and two for > FAT, one for each problem. And make sure to add the MMC custodian > on Cc: > I will, and I will take notice of all other comments (will not answer them all individually). And I guess that I could best post these as new patches (and forget about for this one)? And just in case you haven't noticed: This was my first patch posted... :-) > Best regards, > > Wolfgang Denk > > -- > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: [email protected] > The existence of god implies a violation of causality. > _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
