Re: [PATCH 2/7] get-tar-commit-id: check write_in_full() return against 0

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King wrote:

> We ask to write 41 bytes and make sure that the return value
> is at least 41. This is the same "dangerous" pattern that
> was fixed in the prior commit (wherein a negative return
> value is promoted to unsigned), though it is not dangerous
> here because our "41" is a constant, not an unsigned
> variable.
>
> But we should convert it anyway to avoid modeling a
> dangerous construct.

If the above logic is correct, then I suspect this series does not go
far enough.  write_in_full() would be one of those APIs that is easy
to misuse and difficult to use correctly, and if so we should fix that
at the source instead of trying to teach callers not to hold it wrong.

E.g. what would you think of

 1. Introduce a write_fully (sorry, I am bad at names) function
    that returns 0 on success and a coccinelle semantic patch in
    contrib/coccinelle to migrate callers in "make coccicheck":

@@
expression E;
expression F;
expression G;
@@
-write_in_full(E, F, G) < G
+write_fully(E, F, G)

 2. Run "make coccicheck" and apply the result.
 3. Remove the write_in_full function.

Does read_in_full need a similar treatment?

Thanks,
Jonathan



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux