Fix char_backend_file to manage file handles properly#36
Fix char_backend_file to manage file handles properly#36donghyun-kang-dn wants to merge 1 commit intoqualcomm:mainfrom
Conversation
| } | ||
| } | ||
| fclose(w_file); | ||
| fflush(w_file); // Use fflush instead of fclose |
There was a problem hiding this comment.
I'm not sure the comment makes sense after this patch is merged?
There was a problem hiding this comment.
I see. I'll remove it.
| } | ||
|
|
||
| // write test | ||
| // Original test: send data to the uart at the same time |
There was a problem hiding this comment.
here as well, I don't think we need "Original test:" and "Additional behaviour:" comments ?!
There was a problem hiding this comment.
I see. I'll remove it.
| // Add destructor to clean up file handles | ||
| ~char_backend_file() | ||
| { | ||
| if (w_file != nullptr) { |
There was a problem hiding this comment.
shouldn't we do the same with the r_file as well ?
There was a problem hiding this comment.
I thought that r_file is always handled by rcv_thread().
However, to do same with the r_file at the destructor, we need additional change on the end of rcv_thread():
fclose(r_file);
r_file = nullptr; // required
Is it more reasonable?
There was a problem hiding this comment.
I reflected it on the recent change. (also replaced nullptr to NULL)
- Replace fclose with fflush in the write method to ensure data is flushed to the file. - Implement a destructor to safely close the file handle if it is not null, preventing potential memory leaks. - Update file-backend-test to clarify the original and additional behavior of data writing to the UART. Signed-off-by: Donghyun Kang <donghyun.kang@dnotitia.com>
d5c2d4f to
2dcd08c
Compare
Description: This PR fixes an issue where char_backend_file closes the file handle immediately after the first transaction, causing subsequent writes to fail. (#35 )
Changes:
char_backend_file.h: Replaced fclose with fflush in the writefn method to ensure data is flushed without closing the stream.file-backend-test.cc: Updated the test case to verify continuous data writing to the UART, clarifying both original and extended behaviors.Build & Test Environment Note:
I verified this fix based on the v5.0.1 (latest stable) tag.
I attempted to test on main and v5.0.2, but I could not proceed because the required dependency libqemu-v10.1-v0.3 does not seem to be available in the public QEMU repository (github.com/quic/qemu).
Therefore, the verification was performed in the v5.0.1 environment, but the logic should be compatible with the latest branch.