Hi,
I have just compiled MIDAS svn 4132 on a fresh SuSE 10.3 x86_64 system and gcc
found a bunch of bugs, I guess.
> uname -a
Linux pc 2.6.22.17-0.1-default #1 SMP 2008/02/10 20:01:04 UTC x86_64 x86_64
x86_64 GNU/Linux
> gcc --version
gcc (GCC) 4.2.1 (SUSE Linux)
I see loads of warnings during compile, most of which I know from earlier
compiles:
* warning: dereferencing type-punned pointer will break strict-aliasing rules
* warning: pointer targets in passing argument 3 of 'getsockname' differ in
signedness
But then there is a new one (in fact lots of this one), and brief
inspection suggests this is a true bug with the possibility of truly
nasty consequences.
(1)=========================
src/midas.c:7398: warning: array subscript is above array bounds
Inspection of midas.c:
if (i == MAX_DEFRAG_EVENTS) {
/* no buffer available -> no first fragment received */
7398: free(defrag_buffer[i].pevent);
memset(&defrag_buffer[i].event_id, 0, sizeof(EVENT_DEFRAG_BUFFER));
cm_msg(MERROR, "bm_defragement_event",
"Received fragment without first fragment (ID %d) Ser#:%d",
pevent->event_id & 0x0FFF, pevent->serial_number);
return;
}
And midas.c line 7297:
EVENT_DEFRAG_BUFFER defrag_buffer[MAX_DEFRAG_EVENTS];
So, if(i==MAX_DEFRAG_EVENTS) free(defrag_buffer[i]).
I guess this is an off-by-one bug.
(2)==========================
src/midas.c:2958: warning: array subscript is above array bounds
for (i = 0; i < 13; i++)
2958 if (trans_name[i].transition == transition)
break;
Holy cow, hard-coded "13" in the code! Should be a #define, shouldn't it?
Now look at midas.c lines 94ff:
struct {
int transition;
char name[32];
} trans_name[] = {
{
TR_START, "START",}, {
TR_STOP, "STOP",}, {
TR_PAUSE, "PAUSE",}, {
TR_RESUME, "RESUME",}, {
TR_DEFERRED, "DEFERRED",}, {
0, "",},};
There is no trans_name[12].
The trans_name[12] shows up in line 2894 and 2790, too.
(3)=============================
mfe.c:
src/mfe.c:412: warning: array subscript is above array bounds
src/mfe.c:311: warning: array subscript is above array bounds
src/mfe.c:340: warning: array subscript is above array bounds
412: device_drv->dd(CMD_GET_DEMAND, device_drv->dd_info, i,
&device_drv->mt_buffer->channel[i].array[CMD_GET_DEMAND]);
for (cmd = CMD_GET_FIRST; cmd <= CMD_GET_LAST; cmd++) {
(..)
311: device_drv->mt_buffer->channel[current_channel].array[cmd] = value;
for (cmd = CMD_GET_FIRST; cmd <= CMD_GET_LAST; cmd++) {
(..)
340: device_drv->mt_buffer->channel[i].array[cmd] = value;
CMD_GET_DEMAND is in include/midas.h:
#define CMD_GET_DEMAND CMD_GET_DIRECT // = 20
I haven't even tried to understand mfe.c, nor did I read it.
But I suspect the thing should always be something like
....array[cmd-CMD_GET_FIRST]
so array[] is indexed from [0], not from an arbitrary number that
depends on the number of commands you insert before line 698 in
midas.h. But please could the author of this check this very carefully?
(4)=========================
src/lazylogger.c:1957: warning: array subscript is below array bounds
if ((channel < 0) && (lazyinfo[channel].hKey != 0))
That is lazyinfo[something below zero].
(5)=============================
More warnings an expert might want to have a look at:
* warning: deprecated conversion from string constant to 'char*'
* src/fal.c:106: warning: non-local variable '<anonymous struct> out_info'
uses anonymous type
* src/fal.c:3064: warning: non-local variable '<anonymous struct> eb' uses
anonymous type
I attach the full output of make.
Could someone knowledgeable please have a look at these warnings and fix them?
They make me a bit nervous when thinking about data integrity, and
there are now so many that they actually start to hide serious stuff
like the ones I presented.
Oh, I got rid of the "dereferencing type-punned pointer" thing by adding
"-fno-strict-aliasing" as a compiler flag. Was suggested on the Web. Seemed to
have worked during data taking (the data look reasonable :-). Is that a
possible fix/workaround?
Cheers,
Randolf |