[FFmpeg-devel] [PATCH] avcodec/fft_template: Fix multiple runtime error: signed integer overflow: -1943918714 - 1935113003 cannot be represented in type 'int'

[Michael Niedermayer]

On Fri, Jun 09, 2017 at 09:07:39PM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On Thu, Jun 8, 2017 at 8:57 PM, Michael Niedermayer <michael at niedermayer.cc>
> wrote:
> 
> > On Thu, Jun 08, 2017 at 06:35:07PM -0400, Ronald S. Bultje wrote:
> > > Hi,
> > >
> > > On Thu, Jun 8, 2017 at 6:10 PM, Michael Niedermayer
> > <michael at niedermayer.cc>
> > > wrote:
> > >
> > > > Signed value in
> > > > Unsigned
> > > > INTeger type
> > >
> > > [..]
> > > > Both SUINT and unsigned should produce identical binaries
> > >
> > > This seems to go against the rule that code should be as simple as
> > possible.
> > >
> > > Unsigned is simpler than SUINT if the outcome is the same.
> >
> > You can simply add the part of my mail here as awnser that you snipped
> > away:
> >
> > "But it makes the code hard to understand and maintain because these
> >  values are not positive integers but signed integers. Which for
> >  C standard compliance need to be stored in a unsigned type."
> >
> > A type that avoids the undefinedness of signed but is semantically
> > signed is correct, unsigned is not.
> >
> > If understandable code and maintainable code has no value to you,
> > you would favour using single letter variables exclusivly and would
> > never use typedef.
> > But you do not do that.
> >
> > I fail to understand why you insist on using unsigned in place of a
> > more specific type, it is not the correct nor clean thing to do.
> 
> 
> It's not just me, it appears to be most of us. Can't you just step back at
> some point and be like "ok, I'll let the majority have their way"?

I do not know what the majority prefers. What i see is that the
people objecting are always the same 3-4 people. And very often
they have no authorship or past activity in the code a patch is about.
At least none i could find quickly.
To me that makes these objections seem a bit out of place, more so
because i just dont "get it" why people are against using a more
specific type.

The majority might prefer either way, i have no idea ...

I very much want to choose the types and style the people activly
working on some code prefer. But when i look at git shortlog of code
and look at the copyright/author statments and look at MAINTAINERs and
compare to the names in a thread i often find no real match.

Also as i said, i belive a more specific type makes maintaince
easier, thats why i want to use it in code i maintain. In code others
maintain, if they see no value in it theres really alot less an
argument to use it.

with a specific type in this case here one can add a line and test if
the FFT overflows and where it does so, if it ever produces a bad
result for a real world file. With just unsigned theres no easy way
to find a overflow, one has to painstakingly test this line by line by
hand.

If the people who intend to debug such issues see no value in a more
specific type, no sense in using one.

Iam fighting on this issue because i see this pushing FFmpeg into a
direction where the code is harder to understand and harder to maintain
and we already have many open bugs

Do the people objecting to SUINT volunteer to do the extra maintaince
work that may be caused by not using it ?

or do they expect the existing maintainers not to use SUINT and then
also do the extra work?
its that 2nd case that offends me as iam one of the people who tries
to maintain some of the code in question (not the fft of this patch
here specifically)

thanks

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170611/59a4f5bf/attachment.sig>