Conversation
hush-soul
changed the title
|
|
||
| if (data->log != NULL) | ||
| fclose(data->log); | ||
|
|
There was a problem hiding this comment.
Could you explain a little bit why we move the code here to other lines?
Is there any reasons for closing the gzip first and then close the log?
There was a problem hiding this comment.
Yes, the order of opening is fopen gzipopen, so I think the order of closing is best gzclose, fclose
| #ifdef DLT_LOGSTORAGE_USE_GZIP | ||
| dlt_vlog(LOG_DEBUG, "%s: Opening GZIP log file\n", __func__); | ||
| config->gzlog = gzdopen(config->fd, mode); | ||
| config->log = file; |
There was a problem hiding this comment.
there should be no need to store config->log here. As per my understanding gzclose() will close the passed fd from gzdopen()
There was a problem hiding this comment.
According to my tests, after gzclose, fclose is also needed. This part of the test is included in #655
There was a problem hiding this comment.
@michael-methner
I did a test to remove fclose, and the results are as follows: fclose is necessary, and the currently submitted code is necessary.
add fclose
delete fclose
| for (i = 0; i < cnt; i++) { | ||
| if (config->gzip_compression) { | ||
| if (config->gzip_compression) { |
There was a problem hiding this comment.
I think allocating the suffix here multiple times in the loops was the only cause for the memory leak. Or is there another memleak fixed in this PR?
There was a problem hiding this comment.
Thank you, I will test this case separately when I have time to see if it matches the guess
There was a problem hiding this comment.
I think allocating the suffix here multiple times in the loops was the only cause for the memory leak. Or is there another memleak fixed in this PR?
After testing, it was found that there are two main reasons for memory leaks: 1. fclose needs to be added 2.Allocate memory in a loop
3 participants
#655