On 08/22, Jakub Kicinski wrote: > Using error() makes it impossible for callers to unwind their > changes. Replace error() calls with proper error handling. > > Signed-off-by: Jakub Kicinski <[email protected]>
Acked-by: Stanislav Fomichev <[email protected]> > --- > .../selftests/drivers/net/hw/ncdevmem.c | 528 ++++++++++++------ > 1 file changed, 364 insertions(+), 164 deletions(-) > > diff --git a/tools/testing/selftests/drivers/net/hw/ncdevmem.c > b/tools/testing/selftests/drivers/net/hw/ncdevmem.c > index 71961a7688e6..e75adfed33ac 100644 > --- a/tools/testing/selftests/drivers/net/hw/ncdevmem.c > +++ b/tools/testing/selftests/drivers/net/hw/ncdevmem.c > @@ -115,6 +115,21 @@ struct memory_provider { > size_t off, int n); > }; > > +static void pr_err(const char *fmt, ...) > +{ > + va_list args; > + > + fprintf(stderr, "%s: ", TEST_PREFIX); > + > + va_start(args, fmt); > + vfprintf(stderr, fmt, args); > + va_end(args); > + > + if (errno != 0) > + fprintf(stderr, ": %s", strerror(errno)); > + fprintf(stderr, "\n"); > +} > + > static struct memory_buffer *udmabuf_alloc(size_t size) > { > struct udmabuf_create create; > @@ -123,27 +138,33 @@ static struct memory_buffer *udmabuf_alloc(size_t size) > > ctx = malloc(sizeof(*ctx)); > if (!ctx) > - error(1, ENOMEM, "malloc failed"); > + return NULL; > > ctx->size = size; > > ctx->devfd = open("/dev/udmabuf", O_RDWR); > - if (ctx->devfd < 0) > - error(1, errno, > - "%s: [skip,no-udmabuf: Unable to access DMA buffer device > file]\n", > - TEST_PREFIX); > + if (ctx->devfd < 0) { > + pr_err("[skip,no-udmabuf: Unable to access DMA buffer device > file]"); > + goto err_free_ctx; > + } > > ctx->memfd = memfd_create("udmabuf-test", MFD_ALLOW_SEALING); > - if (ctx->memfd < 0) > - error(1, errno, "%s: [skip,no-memfd]\n", TEST_PREFIX); > + if (ctx->memfd < 0) { > + pr_err("[skip,no-memfd]"); > + goto err_close_dev; > + } > > ret = fcntl(ctx->memfd, F_ADD_SEALS, F_SEAL_SHRINK); > - if (ret < 0) > - error(1, errno, "%s: [skip,fcntl-add-seals]\n", TEST_PREFIX); > + if (ret < 0) { > + pr_err("[skip,fcntl-add-seals]"); > + goto err_close_memfd; > + } > > ret = ftruncate(ctx->memfd, size); > - if (ret == -1) > - error(1, errno, "%s: [FAIL,memfd-truncate]\n", TEST_PREFIX); > + if (ret == -1) { > + pr_err("[FAIL,memfd-truncate]"); > + goto err_close_memfd; > + } > > memset(&create, 0, sizeof(create)); > > @@ -151,15 +172,29 @@ static struct memory_buffer *udmabuf_alloc(size_t size) > create.offset = 0; > create.size = size; > ctx->fd = ioctl(ctx->devfd, UDMABUF_CREATE, &create); > - if (ctx->fd < 0) > - error(1, errno, "%s: [FAIL, create udmabuf]\n", TEST_PREFIX); > + if (ctx->fd < 0) { > + pr_err("[FAIL, create udmabuf]"); > + goto err_close_fd; > + } > > ctx->buf_mem = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, > ctx->fd, 0); > - if (ctx->buf_mem == MAP_FAILED) > - error(1, errno, "%s: [FAIL, map udmabuf]\n", TEST_PREFIX); > + if (ctx->buf_mem == MAP_FAILED) { > + pr_err("[FAIL, map udmabuf]"); > + goto err_close_fd; > + } > > return ctx; > + > +err_close_fd: > + close(ctx->fd); > +err_close_memfd: > + close(ctx->memfd); > +err_close_dev: > + close(ctx->devfd); > +err_free_ctx: > + free(ctx); > + return NULL; > } > > static void udmabuf_free(struct memory_buffer *ctx) > @@ -217,7 +252,7 @@ static void print_nonzero_bytes(void *ptr, size_t size) > putchar(p[i]); > } > > -void validate_buffer(void *line, size_t size) > +int validate_buffer(void *line, size_t size) > { > static unsigned char seed = 1; > unsigned char *ptr = line; > @@ -232,8 +267,10 @@ void validate_buffer(void *line, size_t size) > "Failed validation: expected=%u, actual=%u, > index=%lu\n", > expected, ptr[i], i); > errors++; > - if (errors > 20) > - error(1, 0, "validation failed."); > + if (errors > 20) { > + pr_err("validation failed"); > + return -1; > + } > } > seed++; > if (seed == do_validation) > @@ -241,6 +278,7 @@ void validate_buffer(void *line, size_t size) > } > > fprintf(stdout, "Validated buffer\n"); > + return 0; > } > > static int rxq_num(int ifindex) > @@ -279,7 +317,7 @@ static int rxq_num(int ifindex) > system(command); \ > }) > > -static int reset_flow_steering(void) > +static void reset_flow_steering(void) > { > /* Depending on the NIC, toggling ntuple off and on might not > * be allowed. Additionally, attempting to delete existing filters > @@ -292,7 +330,6 @@ static int reset_flow_steering(void) > run_command( > "ethtool -n %s | grep 'Filter:' | awk '{print $2}' | xargs -n1 > ethtool -N %s delete >&2", > ifname, ifname); > - return 0; > } > > static const char *tcp_data_split_str(int val) > @@ -354,6 +391,11 @@ static int configure_rss(void) > return run_command("ethtool -X %s equal %d >&2", ifname, start_queue); > } > > +static void reset_rss(void) > +{ > + run_command("ethtool -X %s default >&2", ifname, start_queue); > +} > + > static int configure_channels(unsigned int rx, unsigned int tx) > { > struct ethtool_channels_get_req *gchan; > @@ -479,6 +521,7 @@ static int bind_rx_queue(unsigned int ifindex, unsigned > int dmabuf_fd, > > *ys = ynl_sock_create(&ynl_netdev_family, &yerr); > if (!*ys) { > + netdev_queue_id_free(queues); Funny how you spotted this.. Ownership of these is complicated with ynl :-(
