@MSGID: 1@dont-email.me> f56398d4
@REPLY: <yga4jjvik9o.fsf@akutech.de> 88085ea1
@REPLYADDR The Natural Philosopher
<tnp@invalid.invalid>
@REPLYTO 2:5075/128 The Natural Philosopher
@CHRS: CP866 2
@RFC: 1 0
@RFC-Message-ID: 1@dont-email.me>
@RFC-References: 1@dont-email.me>
<ygamsxoixhx.fsf@akutech.de> 4@dont-email.me> <ygail8biyxm.fsf@akutech.de>
1@dont-email.me> <ygaedizitb9.fsf@akutech.de> 2@dont-email.me>
<yga4jjvik9o.fsf@akutech.de>
@TZUTC: 0100
@PID: Mozilla/5.0 (X11; Linux x86_64; rv:102.0)
Gecko/20100101 Thunderbird/102.15.1
@TID: FIDOGATE-5.12-ge4e8b94
On 15/09/2023 15:27, Ralf Fassel wrote:
> * The Natural Philosopher <tnp@invalid.invalid>
> | > | thermometers[i].name=strdup(p); //
> | > | make a copy of the name and attach it
> | > | to our thermometer structure
> | > Memory leak if thermometers[i].name already contains something.
> | >
> | further up the line...
>>
> | bzero(filbuf,sizeof(filbuf));
> | /** first thing to do is clean any allocated memory used to
> | store values. **/
> | for(i=0;i > | free(thermometers[i].name);
> 
> Note that the assignment
> 
>    thermometers[i].name=strdup(p);
> 
> is *inside* the while() loop without a free().
> 
> Probably you argue that there ever is only a single file to read in that
> dir anyway...  Personally, I`ve been bitten by such assumptions, so I`d
> rather check once too often than hunting down "can`t happen" bugs.
> 
> R`
>    
No. you have misunderstood how the code works.

There are up to 4 (NUMBER_RELAYS) thermometer files in that dir, and all 
of them are read in the loop. What there shouldn`t be is more than one 
file with a ZONE number the same.  So no pointer gets more than one STRDUP

If there were, it might be possible to strdup the same pointer twice. 
And the daemon would get a memory leak and crash.

(It would be trivial to simply add a conditional that only strdups to a 
pointer if it is NULL).

That is a possibility that could be caused by mis-configuration of the 
thermometers themselves.

However they are not at this time misconfigured, so it shouldn`t be the 
crash problem, although it is an issue I will consider because fat 
fingers *could* cause it.

I do think that what has happened is that a valid file name has been 
found with empty data, or no file at all,   and then no strdup is done - 
  but the free is, next time around.

That should never happen of course, as the fopen/fwrite sequence should 
certainly not delete the filename, but it is entirely possible that a 
the fopen *truncates* its data. At which point we cant strdup anything, 
so the next free gets a woopsie


Setting the pointers to NULL after free() is nice defensive coding

As is allocating memory only if the pointers are null.

So both are in there now.


-- 
"Progress is precisely that which rules and regulations did not foresee,"

  - Ludwig von Mises

 --- Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
Thunderbird/102.15.1
 * Origin: A little, after lunch (2:5075/128)
SEEN-BY: 5001/100 5005/49 5015/255 5019/40 5020/715
848 1042 4441 12000
SEEN-BY: 5030/49 1081 5058/104 5075/128
@PATH: 5075/128 5020/1042 4441