Re: [LTP] [PATCH v5] fanotify22.c: handle multiple asynchronous error events

[Andrea Cervesato via ltp <ltp@lists.linux.it>]

Hi Wei,

The overall approach with poll/read loop and consolidate_events() looks
good. A few issues below.

> +		char *current_pos = event_buf + read_len;
> +		int ret = read(fd_notify, current_pos, BUF_SIZE - read_len);

read() returns ssize_t, not int. Should be:

		ssize_t ret = read(fd_notify, current_pos, BUF_SIZE - read_len);

Also, both read_len and BUF_SIZE are effectively unsigned here. If
read_len ever reaches or exceeds BUF_SIZE, the subtraction wraps to a
huge value and read() will write past event_buf. A defensive check
before the read would be safer:

		if (read_len >= BUF_SIZE)
			tst_brk(TBROK, "Event buffer full");

> +		if (ret < 0) {
> +			tst_res(TFAIL, "%s: read failed: %s", tcase->name, strerror(errno));
> +			goto out;
> +		}

Two things here:

1) Use TERRNO instead of manual strerror(errno):

		tst_res(... | TERRNO, "%s: read failed", tcase->name);

2) A read() failure after poll() confirmed data availability is a test
   infrastructure problem, not a test logic failure. This should be
   TBROK, not TFAIL:

		tst_brk(TBROK | TERRNO, "%s: read failed", tcase->name);

Regards,
--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp