[FFmpeg-devel] [PATCH 001/244] Add a new channel layout API

[Nicolas George]

Hi.

Anton Khirnov (12019-12-10):
> I disagree. I think I have a fair amount of skill and experience as a C
> developer, but I still get hit by those issues frequently. It's extra
> trouble for only theoretical gain.

I concede this to you, except for the last sentence: I do not agree that
the gain is only theoretical. We only need to look at the number of
recent commits fixing possible integer overflows. Signed arithmetic is
tricky, possibly as tricky as the traps of mixing signed and unsigned.
The reason we are seeing this only now is because compilers have only
recently started to optimize these undefined behaviors aggressively. And
we have only recently started to care. But these are real traps that do
not happen with unsigned.

I would advise everybody to adopt the policy of always using unsigned
unless there is a good reason to use signed, instead of the default now
which is the opposite.

Of course, for your own code and everybody else, I would not block the
patch if my advice is not followed. But this is public API, it should be
clean, so I will insist a little more.

Using the logical type has practical benefits. One is that it guarantees
to the API users that negative values cannot happen, need not to be
tested. With an unsigned, the value can be injected in size and pointer
arithmetic directly. With a signed, the API user need either to add an
useless test or to blindly trust the library. Blindly trusting a piece
of code should always put people ill at ease.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20191217/230de6d8/attachment.sig>