* [PATCH] Modify the return value ret to EOPNOTSUPP when initialized to reduce repeated assignment of errno @ 2022-08-11 7:56 Zhang chunchao 2022-08-11 15:02 ` Stefano Garzarella 0 siblings, 1 reply; 4+ messages in thread From: Zhang chunchao @ 2022-08-11 7:56 UTC (permalink / raw) To: axboe, asml.silence; +Cc: io-uring, linux-kernel, kernel, Zhang chunchao Remove unnecessary initialization assignments. Signed-off-by: Zhang chunchao <[email protected]> --- io_uring/io_uring.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c index b54218da075c..8c267af06401 100644 --- a/io_uring/io_uring.c +++ b/io_uring/io_uring.c @@ -3859,14 +3859,13 @@ SYSCALL_DEFINE4(io_uring_register, unsigned int, fd, unsigned int, opcode, void __user *, arg, unsigned int, nr_args) { struct io_ring_ctx *ctx; - long ret = -EBADF; + long ret = -EOPNOTSUPP; struct fd f; f = fdget(fd); if (!f.file) return -EBADF; - ret = -EOPNOTSUPP; if (!io_is_uring_fops(f.file)) goto out_fput; -- 2.18.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Modify the return value ret to EOPNOTSUPP when initialized to reduce repeated assignment of errno 2022-08-11 7:56 [PATCH] Modify the return value ret to EOPNOTSUPP when initialized to reduce repeated assignment of errno Zhang chunchao @ 2022-08-11 15:02 ` Stefano Garzarella 2022-08-11 15:41 ` Jens Axboe 0 siblings, 1 reply; 4+ messages in thread From: Stefano Garzarella @ 2022-08-11 15:02 UTC (permalink / raw) To: Zhang chunchao; +Cc: axboe, asml.silence, io-uring, linux-kernel, kernel On Thu, Aug 11, 2022 at 03:56:38PM +0800, Zhang chunchao wrote: >Remove unnecessary initialization assignments. > >Signed-off-by: Zhang chunchao <[email protected]> >--- > io_uring/io_uring.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > >diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c >index b54218da075c..8c267af06401 100644 >--- a/io_uring/io_uring.c >+++ b/io_uring/io_uring.c >@@ -3859,14 +3859,13 @@ SYSCALL_DEFINE4(io_uring_register, unsigned int, fd, unsigned int, opcode, > void __user *, arg, unsigned int, nr_args) > { > struct io_ring_ctx *ctx; >- long ret = -EBADF; >+ long ret = -EOPNOTSUPP; > struct fd f; > > f = fdget(fd); > if (!f.file) > return -EBADF; > >- ret = -EOPNOTSUPP; > if (!io_is_uring_fops(f.file)) > goto out_fput; > What about remove the initialization and assign it in the if branch? I find it a bit easier to read. I mean something like this: --- a/io_uring/io_uring.c +++ b/io_uring/io_uring.c @@ -3859,16 +3859,17 @@ SYSCALL_DEFINE4(io_uring_register, unsigned int, fd, unsigned int, opcode, void __user *, arg, unsigned int, nr_args) { struct io_ring_ctx *ctx; - long ret = -EBADF; + long ret; struct fd f; f = fdget(fd); if (!f.file) return -EBADF; - ret = -EOPNOTSUPP; - if (!io_is_uring_fops(f.file)) + if (!io_is_uring_fops(f.file)) { + ret = -EOPNOTSUPP; goto out_fput; + } ctx = f.file->private_data; Otherwise remove the initialization, but leave the assignment as it is now. Thanks, Stefano ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Modify the return value ret to EOPNOTSUPP when initialized to reduce repeated assignment of errno 2022-08-11 15:02 ` Stefano Garzarella @ 2022-08-11 15:41 ` Jens Axboe 2022-08-11 18:42 ` Stefano Garzarella 0 siblings, 1 reply; 4+ messages in thread From: Jens Axboe @ 2022-08-11 15:41 UTC (permalink / raw) To: Stefano Garzarella, Zhang chunchao Cc: asml.silence, io-uring, linux-kernel, kernel On 8/11/22 9:02 AM, Stefano Garzarella wrote: > On Thu, Aug 11, 2022 at 03:56:38PM +0800, Zhang chunchao wrote: >> Remove unnecessary initialization assignments. >> >> Signed-off-by: Zhang chunchao <[email protected]> >> --- >> io_uring/io_uring.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c >> index b54218da075c..8c267af06401 100644 >> --- a/io_uring/io_uring.c >> +++ b/io_uring/io_uring.c >> @@ -3859,14 +3859,13 @@ SYSCALL_DEFINE4(io_uring_register, unsigned int, fd, unsigned int, opcode, >> void __user *, arg, unsigned int, nr_args) >> { >> struct io_ring_ctx *ctx; >> - long ret = -EBADF; >> + long ret = -EOPNOTSUPP; >> struct fd f; >> >> f = fdget(fd); >> if (!f.file) >> return -EBADF; >> >> - ret = -EOPNOTSUPP; >> if (!io_is_uring_fops(f.file)) >> goto out_fput; >> > > What about remove the initialization and assign it in the if branch? > I find it a bit easier to read. > > I mean something like this: > > --- a/io_uring/io_uring.c > +++ b/io_uring/io_uring.c > @@ -3859,16 +3859,17 @@ SYSCALL_DEFINE4(io_uring_register, unsigned int, fd, unsigned int, opcode, > void __user *, arg, unsigned int, nr_args) > { > struct io_ring_ctx *ctx; > - long ret = -EBADF; > + long ret; > struct fd f; > > f = fdget(fd); > if (!f.file) > return -EBADF; > > - ret = -EOPNOTSUPP; > - if (!io_is_uring_fops(f.file)) > + if (!io_is_uring_fops(f.file)) { > + ret = -EOPNOTSUPP; > goto out_fput; > + } > > ctx = f.file->private_data; > > > Otherwise remove the initialization, but leave the assignment as it is now. Generally the kernel likes to do: err = -EFOO; if (something) goto err_out; rather than put it inside the if clause. I guess the rationale is it makes it harder to forget to init the error value. I don't feel too strongly, I'm fine with your patch too. Can you send it as a real patch? -- Jens Axboe ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Modify the return value ret to EOPNOTSUPP when initialized to reduce repeated assignment of errno 2022-08-11 15:41 ` Jens Axboe @ 2022-08-11 18:42 ` Stefano Garzarella 0 siblings, 0 replies; 4+ messages in thread From: Stefano Garzarella @ 2022-08-11 18:42 UTC (permalink / raw) To: Jens Axboe, Zhang chunchao Cc: Zhang chunchao, asml.silence, io-uring, linux-kernel, kernel On Thu, Aug 11, 2022 at 09:41:38AM -0600, Jens Axboe wrote: >On 8/11/22 9:02 AM, Stefano Garzarella wrote: >> On Thu, Aug 11, 2022 at 03:56:38PM +0800, Zhang chunchao wrote: >>> Remove unnecessary initialization assignments. >>> >>> Signed-off-by: Zhang chunchao <[email protected]> >>> --- >>> io_uring/io_uring.c | 3 +-- >>> 1 file changed, 1 insertion(+), 2 deletions(-) >>> >>> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c >>> index b54218da075c..8c267af06401 100644 >>> --- a/io_uring/io_uring.c >>> +++ b/io_uring/io_uring.c >>> @@ -3859,14 +3859,13 @@ SYSCALL_DEFINE4(io_uring_register, unsigned int, fd, unsigned int, opcode, >>> void __user *, arg, unsigned int, nr_args) >>> { >>> struct io_ring_ctx *ctx; >>> - long ret = -EBADF; >>> + long ret = -EOPNOTSUPP; >>> struct fd f; >>> >>> f = fdget(fd); >>> if (!f.file) >>> return -EBADF; >>> >>> - ret = -EOPNOTSUPP; >>> if (!io_is_uring_fops(f.file)) >>> goto out_fput; >>> >> >> What about remove the initialization and assign it in the if branch? >> I find it a bit easier to read. >> >> I mean something like this: >> >> --- a/io_uring/io_uring.c >> +++ b/io_uring/io_uring.c >> @@ -3859,16 +3859,17 @@ SYSCALL_DEFINE4(io_uring_register, unsigned int, fd, unsigned int, opcode, >> void __user *, arg, unsigned int, nr_args) >> { >> struct io_ring_ctx *ctx; >> - long ret = -EBADF; >> + long ret; >> struct fd f; >> >> f = fdget(fd); >> if (!f.file) >> return -EBADF; >> >> - ret = -EOPNOTSUPP; >> - if (!io_is_uring_fops(f.file)) >> + if (!io_is_uring_fops(f.file)) { >> + ret = -EOPNOTSUPP; >> goto out_fput; >> + } >> >> ctx = f.file->private_data; >> >> >> Otherwise remove the initialization, but leave the assignment as it is now. > >Generally the kernel likes to do: > >err = -EFOO; >if (something) > goto err_out; > >rather than put it inside the if clause. I guess the rationale is it >makes it harder to forget to init the error value. I don't feel too ah, thanks for pointing this out! Make sense to me, but I hope recent compilers can spot that kind of issue :-) >strongly, I'm fine with your patch too. Can you send it as a real patch? @Zhang: if you want, feel free to change your patch following the suggestions and send a new version, otherwise I can send mine of course. Thanks, Stefano ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-08-11 18:42 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-08-11 7:56 [PATCH] Modify the return value ret to EOPNOTSUPP when initialized to reduce repeated assignment of errno Zhang chunchao 2022-08-11 15:02 ` Stefano Garzarella 2022-08-11 15:41 ` Jens Axboe 2022-08-11 18:42 ` Stefano Garzarella
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox