ret = i2c_smbus_write_byte_data(client, offset, value);
- if (ret < 0) {
dev_err(ctx->dev, "writebm: %4s[0x%02x] <- 0x%02x\n",sii9234_client_name[id], offset, value);ctx->i2c_error = ret;- }
- if (!ret)
return 0;Ugh. No. Don't do success handling on the last if statement.
I find my approach useful in this case.
Also while I personally prefer testing for non-zero,
I got used to this checking style to some degree.
the ALSA people got annoyed at you for changing tests for < 0
It seems that involved software developers have got special preferences there.
but you're doing it again.
I dared to propose such an adjustment once more. Would you like discuss corresponding reasons any further?
And it introduces a bug,
Unfortunately, a hiccup in my software development attention …
although I see now that you fixed it in v2.
Thanks that you noticed also this small update.
https://patchwork.kernel.org/patch/10021767/ https://lkml.kernel.org/r/2ecf0bb7-7129-40e4-cefc-0bc2d0f7ee8b@users.sourceforge.net
I can't get excited about these sort of risky low value patches.
I try again to point special software improvement opportunities out.
+report_failure:
- dev_err(ctx->dev, "writebm: %4s[0x%02x] <- 0x%02x\n",
sii9234_client_name[id], offset, value);- ctx->i2c_error = ret; return ret;
}
How do you think about to move this source code to the end of this function implementation?
Regards, Markus