axboe / fio

Flexible I/O Tester
GNU General Public License v2.0
5.21k stars 1.26k forks source link

iolog, dataplacement: fix coverity scan defect #1767

Closed parkvibes closed 4 months ago

parkvibes commented 4 months ago

iolog: fix Error handling issues (NEGATIVE_RETURNS)

CID 494151: Error handling issues (NEGATIVE_RETURNS) @ io_u.c:1877 in get_io_u() This patch removes negative returns from dp_init() to ensure its value can be properly consumed by td_verror()

Signed-off-by: Hyunwoo Park dshw.park@samsung.com

iolog: fix Null pointer dereferences (FORWARD_NULL)

CID 494150: Null pointer dereferences (FORWARD_NULL) @ iolog.c:148 in ipo_special() This patch removes the possibility of null pointer dereferencing(io_u->file) throughout the call stack of get_io_u() → read_iolog_get() → dp_fill_dspec_data()

Signed-off-by: Hyunwoo Park dshw.park@samsung.com

vincentkfu commented 4 months ago

Please change the first lines of the commit messages to describe the actual issue instead of just writing that the issue was identified by coverity.

vincentkfu commented 4 months ago

For the negative errno issue I would much rather see code that does something like what zbd.c:zbd_init_zone_info() does:

        ret = zbd_create_zone_info(td, file);
        if (ret < 0)
                td_verror(td, -ret, "zbd_create_zone_info() failed");

Just negate the value sent to td_verror(). This involves fewer code changes than your patch.

I do not know the full history of returning negative errno values but this is a common pattern: https://stackoverflow.com/questions/36845866/history-of-using-negative-errno-values-in-gnu

parkvibes commented 4 months ago

As dp_init() can return a positive value, I made dp_init_ret an absolute value using abs(dp_init_ret) instead of -dp_init_ret