Re: [PATCH 0/7] trace-cmd: Fix misc issues uncoverd by static analysis

[Jerome Marchand <jmarchan@redhat.com>]

On 18/12/2024 23:53, Steven Rostedt wrote:
> On Thu,  5 Dec 2024 15:44:32 +0100
> "Jerome Marchand" <jmarchan@redhat.com> wrote:
> 
>> More issues were found by running static analysers on the code
>> of trace-cmd with openscanhub[1].
>>
>> [1] https://fedoraproject.org/wiki/OpenScanHub
>>
>> Jerome Marchand (7):
>>    trace-cmd lib: Close FDs in create_buffer_recorder_fd2 it allocation
>>      fails
>>    trace-cmd lib: Prevent a memory leak in tracecmd_tsync_with_guest()
>>    trace-cmd lib: Prevent a leaked FD in
>>      __tracecmd_create_buffer_recorder()
>>    trace-cmd lib: Prevent memory leak in tracecmd_msg_wait_for_cmd()
>>    trace-cmd sqlhist: Initialize name in trace_sqlhist()
>>    trace-cmd: Fix memory leak in stop_mapping_vcpus()
>>    trace-cmd record: Fix stdin redirection to /dev/null
>>
> 
> I'm going to apply all your patches except the two that deal with file
> descriptors. The functions should not be closing file descriptors that they
> did not open. The real fix there is to remove where it does close the file
> descriptors and move the closing in the error paths of the callers.
> 
> I must have gotten lazy and just let the functions do the closing when they
> failed, but that is just prone to bugs.

That makes sense. I'll send updated fixes after the holidays break.

Thanks,
Jerome

> 
> -- Steve
> 
> 
>>   lib/trace-cmd/trace-msg.c      |  4 +++-
>>   lib/trace-cmd/trace-recorder.c | 19 ++++++++++---------
>>   lib/trace-cmd/trace-timesync.c |  1 +
>>   tracecmd/trace-record.c        |  2 +-
>>   tracecmd/trace-sqlhist.c       |  2 +-
>>   tracecmd/trace-tsync.c         |  1 +
>>   6 files changed, 17 insertions(+), 12 deletions(-)
>>
>