Issue 33781: audioop.c: fbound() casts double to int for its return value (original) (raw)
Created on 2018-06-06 10:23 by vstinner, last changed 2022-04-11 14:59 by admin. This issue is now closed.
Messages (12)
Author: STINNER Victor (vstinner) *
Date: 2018-06-06 10:23
Extract of Python 2.7, Modules/audioop.c:
static int fbound(double val, double minval, double maxval) { if (val > maxval) val = maxval; else if (val < minval + 1) val = minval; return val; }
Example of usage:
double factor, fval, maxval, minval;
int len, size, val = 0;
fval = (double)val*factor;
val = (int)floor(fbound(fval, minval, maxval));
It seems wrong to me to call floor() with an integer. Why fbound() doesn't return a double?
fbound() has been modified to explicitly cast its result to an int in the master branch:
static int fbound(double val, double minval, double maxval) { if (val > maxval) val = maxval; else if (val < minval + 1) val = minval; return (int)val; }
But master still uses something like:
val = floor(fbound(val, minval, maxval));
It seems like fbound() result is always passed into floor(). Maybe floor() should be moved into fbound()?
Attached PR changes fbound() to round correctly: call floor() into fbound().
--
I looked at the code because of the following compiler warning on Python 2.7:
[00:02:21] ..\Modules\audioop.c(38): warning C4244: 'return' : conversion from 'double' to 'int', possible loss of data [C:\projects\cpython\PCbuild\pythoncore.vcxproj]
Author: STINNER Victor (vstinner) *
Date: 2018-06-06 11:02
Oh, fbound() cast to (int) has been done by... myself :-D
commit f2b9a340ef143099726fb642951410e4ad793c7f Author: Victor Stinner <victor.stinner@gmail.com> Date: Tue May 7 23:49:15 2013 +0200
audioop: explicit cast to fix a compiler warning
diff --git a/Modules/audioop.c b/Modules/audioop.c index 7e40bbddc6..4f2b25f33a 100644 --- a/Modules/audioop.c +++ b/Modules/audioop.c @@ -37,7 +37,7 @@ fbound(double val, double minval, double maxval) val = maxval; else if (val < minval + 1) val = minval;
- return val;
- return (int)val;
}
Stupid me!
Author: STINNER Victor (vstinner) *
Date: 2018-06-06 13:50
New changeset 45e4efba7fa2abe61d25e4f8b5bf482e19ff1280 by Victor Stinner in branch 'master': bpo-33781: audioop: enhance rounding double as int (GH-7447) https://github.com/python/cpython/commit/45e4efba7fa2abe61d25e4f8b5bf482e19ff1280
Author: Serhiy Storchaka (serhiy.storchaka) *
Date: 2018-06-06 14:18
It was my mistake. fbound() should be return double.
But since it is always used with floor(), it is better to move floor() inside it.
Author: STINNER Victor (vstinner) *
Date: 2018-06-06 14:23
But since it is always used with floor(), it is better to move floor() inside it.
Yeah, that's what I did ;-) I will backport the change to 3.7, 3.6 and 2.7. My final goal is just to fix a compiler warning in 2.7 :-D
Author: miss-islington (miss-islington)
Date: 2018-06-06 14:33
New changeset 5c022f13ab6db8929e092ad035b3dc61701e3198 by Miss Islington (bot) in branch '3.7': bpo-33781: audioop: enhance rounding double as int (GH-7447) https://github.com/python/cpython/commit/5c022f13ab6db8929e092ad035b3dc61701e3198
Author: STINNER Victor (vstinner) *
Date: 2018-06-06 14:41
Note: I chose to not add a NEWS entry, because even if my change might enhance the rounding, I don't think that it has any effect in practice :-)
Author: STINNER Victor (vstinner) *
Date: 2018-06-06 15:12
New changeset b17d409bc0458e3981987c2358da661f228f5891 by Victor Stinner in branch '2.7': bpo-33781: audioop: enhance rounding double as int (GH-7447) (GH-7452) https://github.com/python/cpython/commit/b17d409bc0458e3981987c2358da661f228f5891
Author: STINNER Victor (vstinner) *
Date: 2018-06-06 15:14
New changeset b17d409bc0458e3981987c2358da661f228f5891 by Victor Stinner in branch '2.7': bpo-33781: audioop: enhance rounding double as int (GH-7447) (GH-7452)
I checked AppVeyor logs: this change fixed the audioop.c warning that I wanted to fix ;-)
Author: STINNER Victor (vstinner) *
Date: 2018-06-06 15:18
New changeset 2bc1946fb02e140f5f5ac21a1afa2763615ad16b by Victor Stinner (Miss Islington (bot)) in branch '3.6': bpo-33781: audioop: enhance rounding double as int (GH-7447) (GH-7451) https://github.com/python/cpython/commit/2bc1946fb02e140f5f5ac21a1afa2763615ad16b
Author: STINNER Victor (vstinner) *
Date: 2018-06-06 15:51
New changeset e5b79c546370521764457ea2ec809303e580f5ea by Victor Stinner in branch '2.7': bpo-19418: audioop.c: Fix warnings on -0x80000000 (GH-7453) https://github.com/python/cpython/commit/e5b79c546370521764457ea2ec809303e580f5ea
Author: STINNER Victor (vstinner) *
Date: 2018-06-06 17:04
Ok, fbound() is now better in all branches. I also fixed all compiler warnings in 2.7. I close the issue.
History
Date
User
Action
Args
2022-04-11 14:59:01
admin
set
github: 77962
2018-06-06 17:04:01
vstinner
set
status: open -> closed
resolution: fixed
messages: +
stage: patch review -> resolved
2018-06-06 15:51:10
vstinner
set
messages: +
2018-06-06 15🔞30
vstinner
set
messages: +
2018-06-06 15:17:49
vstinner
set
pull_requests: + <pull%5Frequest7076>
2018-06-06 15:14:11
vstinner
set
messages: +
2018-06-06 15:12:43
vstinner
set
messages: +
2018-06-06 14:41:36
vstinner
set
messages: +
2018-06-06 14:40:41
vstinner
set
pull_requests: + <pull%5Frequest7074>
2018-06-06 14:33:08
miss-islington
set
nosy: + miss-islington
messages: +
2018-06-06 14:23:20
vstinner
set
messages: +
2018-06-06 14🔞13
miss-islington
set
pull_requests: + <pull%5Frequest7073>
2018-06-06 14🔞13
serhiy.storchaka
set
nosy: + serhiy.storchaka
messages: +
2018-06-06 14:17:16
miss-islington
set
pull_requests: + <pull%5Frequest7072>
2018-06-06 13:50:52
vstinner
set
messages: +
2018-06-06 11:02:16
vstinner
set
messages: +
2018-06-06 10:48:42
vstinner
set
keywords: + patch
stage: patch review
pull_requests: + <pull%5Frequest7070>
2018-06-06 10:23:23
vstinner
create