This is the mail archive of the ecos-patches@sources.redhat.com mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Fix for misc compiler warnings




Andrew Lunn wrote:

On Wed, Oct 13, 2004 at 08:40:44PM -0700, David Brennan wrote:


Yes I agree that casting i82559_heap_free to an int should be enough. I am using gcc 3.2.3. And I guess it is possible that the gcc folks made a mistake with this warning. However, I submitted this to get rid of it.


What exactly is the warning?


$ make -s
headers finished
/ecos-c/cygwin/opt/ecos/ecos-cvs/ecos/packages/devs/eth/intel/i82559/current/src/if_i82559.c: In function `pciwindow_mem_alloc':
/ecos-c/cygwin/opt/ecos/ecos-cvs/ecos/packages/devs/eth/intel/i82559/current/src/if_i82559.c:1122: warning: comparison between pointer and integer
/ecos-c/cygwin/opt/ecos/ecos-cvs/ecos/packages/devs/eth/intel/i82559/current/src/if_i82559.c:1122: warning: comparison between pointer and integer
/ecos-c/cygwin/opt/ecos/ecos-cvs/ecos/packages/devs/eth/intel/i82559/current/src/if_i82559.c:1122: warning: comparison between pointer and integer
build finished




Ok, the whole point of the CYG_CHECK is to protect programmers from themselves. And I agree tat the first check might not be necessary. But I am trying to get my i386-pc ide driver working on a different machine than my usual target. I am testing it with fs/fat/fileio1.c. If the mount fails, (which it currently is on my hardware), and you try and getcwd, you will get a SIGILL because mte->fs is null. (I originally thought the roblem was that mte was NULL, that is the reason the first is in there.) So I think there is value in the second check, but if you insist on removing the first, I am ok with that.



Humm. I would say you are actually checking in the wrong place. The root problem here is that mount() failed. That probably means virtually every other filesystem system call will then fail in some way or other. Picking out just getcwd() for a test is not particularly robust since most people will actually call open() next. I would say the assert really belongs in your application checking the return code from mount. If i were going to add an assert for mts->fs i would make it part of LOCK_FS().



Ok, I'll put it in there. I did not originally read the LOCK_FS code, since I assumed it would just lock the file system. After looking at the code, it makes sense to put the mte->fs check there. I'm kind of in the middle of some other changes. But I will hopefully be able to submit a patch shortly.

David


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]