µTasker Forum

µTasker Forum => utFAT => Topic started by: dkg on February 21, 2011, 05:45:28 PM

Title: Gnu compiler warnings in mass_storage.c
Post by: dkg on February 21, 2011, 05:45:28 PM
Hi Mark,

I am finally getting the new code integrated into my development project and I got a warning (among many) on compiling mass_storage.c. The line I am concerned about is in fnCreateNameParagraph and looks like this:

Quote
if ((iLength == 0) && (cInputCharacter == DIR_NAME_FREE)) {

The warning that I get out of gcc v4.5.1 is: 'comparison is always false due to limited range of data type'. The compiler is warning about the comparison to DIR_NAME_FREE. If I cast the variable to a (unsigned char) the warning goes away. This cast is done all over that routine for other comparisons so I gotta wonder if cInputCharacter shouldn't be replaced with an unsigned char variable instead?

While we are on the subject of gcc warnings, the other one I have always had to "ignore" is the way static structures are initialized in the uTasker code. The initializer list always looks like = {{0}} no matter what the structure contains. The gcc compiler warns about all of these declarations because it believes the initializer list  "must agree, in type and number, with the components of the structure itself". I can get rid of the warning by removing the initializer (the standard says static structures are initialized to zero by default) or by expanding the initializer list to match the structure.

Finally, one other group of warnings I get in mass_storage.c is all the places there is a READ_SPI_DATA without storing the result. I understand the reason for this but the warning implies those lines might  not generate any code (yes, I know QDR is cast as volatile).

Of course these are just warnings and, in most cases, nothing would change in the generated code if you cleared them up. Call me a perfectionist, but I like to create warning-free code since every now and then compiler warnings actually are important to consider. ;)

Dave G.
Title: Re: Gnu compiler warnings in mass_storage.c
Post by: mark on February 21, 2011, 09:23:47 PM
Hi Dave

You get a lot more warings than me when I compile with GCC [gcc version 4.5.1 (Sourcery G++ Lite 2010.09-39)] - did you increase the warning level?

1)
#define DIR_NAME_FREE                    0xe5
CHAR cInputCharacter;
(cInputCharacter == DIR_NAME_FREE)


This is serious. The compiler will take the value in cInputCharacter and extend it to an integer, which will make it 0xffffffe5 on a 32 bit processor, if it had the value 0xe5 in it. Therefore the expression can never be true.
Either casiting the cInputCharacter to unsigned char or DIR_NAME_FREE to CHAR resolves this. I have chosen to use (cInputCharacter == (CHAR)DIR_NAME_FREE)

2) I don't know what the best solution is for the {0} case. GCC will complain if a struct array is simple set to {0}; Using {{0}} quietened GCC in all projects but, presumably, with a higher level of warning is gets more pedantic. Removing the initialiser should of cause be the same as having none - but I have avoided any un-initialised structs to make it obviouls that they are initialised with 0. Maybe there is a GCC pragma that can be used to quieten the warnings for all warning levels, based on the fact that with or without the zero initialiser it doesn't actually make any real coding difference (i.e. not dangerous, just annoying).

3) In the Coldfire project:

#define READ_SPI_DATA()            (unsigned char)QDR

The code is thus (unsigned char)*(volatile unsigned short *)(QSPI_ADD + 0x14);

I don't know how to quieten the compiler here and have just accepted these warnings. Although the code looks OK (the volatile ensures that the read is made) I have just changed the define to:

#define READ_SPI_DATA()            (volatile unsigned char)QDR

This makes the volatile more readable in both locations.


Regards

Mark

Title: Re: Gnu compiler warnings in mass_storage.c
Post by: dkg on February 21, 2011, 09:49:25 PM
Hi Mark,

Yes, I do tend to maximize my warning levels ;)  I'm sure that would explain the differences. My command line in the makefile results in the following warning settings:

-Wall -Wstrict-prototypes -Wextra -Wno-unused-parameter -Wmissing-prototypes -Wmissing-declarations

1) I thought about casting the constant to (CHAR) but I was following the pattern used in many other spots of that module.

2) I understand the desire to show initializers -- I hate seeing uninitialized variables myself. I added the -Wno-missing-field-initializers option to my command line so it is now happy with the abbreviated initializer (it was the -Wextra option that caused the warnings).

3) For the unused value warnings, I suppose we could use a pragma push/pop and a pragma to set the -Wno-unused-value option around that code. But that gets messy especially if we try to mask warnings around each statement and not the whole routine or module. The other alternative is to set a variable with the value but then that will probably generate extra code we don't need or want. So, for now, I guess I'll ignore the warnings :-/

Dave G.