On 14.04.22 10:21, Dan Carpenter wrote: > On Thu, Apr 14, 2022 at 09:31:56AM +0200, Oliver Neukum wrote: >> >> On 13.04.22 17:32, Dan Carpenter wrote: >>> Bug: buffer partially filled. Information leak. >>> >>> If you return the bytes then the only correct way to write error >>> handling is: >>> >>> if (ret < 0) >>> return ret; >>> if (ret != size) >>> return -EIO; >>> >> You have to make up your mind on whether you ever need to read >> answer of a length not known before you try it. The alternative of >> passing a pointer to an integer for length is worse. > How is it worse? Can you give an example, so I will write a static > checker rule for it? My favorite example would be: int usb_bulk_msg(struct usb_device *usb_dev, unsigned int pipe, void *data, int len, int *actual_length, int timeout) The actual_length parameter is horrible. Now, there are situations like this where this pattern is unavoidable, because in addition to an error you can get a partial completion of an operation. Do I see it correctly that you would prefer this pattern even if you could report either an error or a result in the function? > There used to be more APIs that consistently caused bug after bug where > we mixed positives success values with negative error codes. We > converted some bad offenders to return the positive as a parameter > and I was really happy about that. > > Another example I used to see a lot is request_irq() saved to an > unsigned. These days I think GCC warns about that? Maybe the build > bots get to it before I do. > That needs to be checked for.. In fact while we are at it, do we check for integer overflows? Regards Oliver
