On Thu, Mar 16, 2017 at 4:23 PM, Andrey Smirnov
[off-list ref] wrote:

On Tue, Mar 14, 2017 at 4:17 PM, Andy Shevchenko
[off-list ref] wrote:

quoted

On Tue, Mar 14, 2017 at 3:48 PM, Andrey Smirnov
[off-list ref] wrote:

quoted

quoted

+int serdev_device_write(struct serdev_device *serdev,
+                       const unsigned char *buf, size_t count)
+{

quoted

+       int ret = count;

If count by some reason bigger than INT_MAX...

True, I was merely copying type signature of serdev_device_write_buf,
which would have the same problem. I can add an appropriate check to
the code, but at the same time, I'd love to know why it wasn't a
concern in the latter function.

Perhaps we may survive without changing it, just document that count
should not be bigger than INT_MAX.
OTOH it's counter intuitive to have size_t type which is used as int.

quoted

quoted

+       for (;;) {
+               size_t chunk;
+
+               reinit_completion(&serdev->write_wakeup);
+
+               chunk = serdev_device_write_buf(serdev, buf, count);

quoted

+               if (chunk < 0) {

This will never happen. What kind of test did you try?

None of my code using that function ever hit that condition, OTOH, I
am not sure why I should ignore the API's signed return type, which I
assume is so in order to facilitate negative error returns. My
thinking is that even if in the current codebase is incapable of ever
returning negative result today, that doesn't mean it won't ever be,
and IMHO the chances of that are no different from the chances of
someone passing 'count' that is bigger than INT_MAX. Given that I'd
rather keep the check until the type signature of
serdev_device_write_buf changes.

You missed the point

size_t is *unsigned* type!

quoted

quoted

+               if (!count)

What is supposed to be returned? Initial count? Does it make any sense?

I'll change it to do that in v2, but I am open to suggestions.

Since function lacks of description (or I missed it?) I am out of
knowledge what you are trying to achieve here.

I can see how that might make the diff look nice, but OTOH, I'd rather
not limit myself to only ~4 letters.

Up to you and Rob.

-- 
With Best Regards,
Andy Shevchenko