Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

-maxsize no bigger than 2.14748e+09 in [soundfiler] on windows #380

Closed
porres opened this issue Jun 19, 2018 · 11 comments
Closed

-maxsize no bigger than 2.14748e+09 in [soundfiler] on windows #380

porres opened this issue Jun 19, 2018 · 11 comments

Comments

@porres
Copy link
Contributor

porres commented Jun 19, 2018

In windows, [soundfiler] won't accept a -maxsize flag bigger than 2.14748e+09 or something (I got that with trial and error) for some reason.

the console shows a typical syntax error for [soundfiler]:

usage: read [flags] filename tablename...
flags: -skip <n> -resize -maxsize <n> ...
-raw <headerbytes> <channels> <bytespersamp> <endian (b, l, or n)>.
@Spacechild1
Copy link
Contributor

Spacechild1 commented Jun 19, 2018

in soundfiler_read (d_soundfile.c) maxsize is declared as long. on Windows, long is 32 bit in both 32 bit and 64 bit programs and your number exceeds the maximum positive size (2^31). although signed integer overflow is strictly undefined behaviour, it will most often wrap around, so you end up with a negative number. soundfiler_read fails if maxsize is negative:

if (argc < 2 || argv[1].a_type != A_FLOAT ||
                ((maxsize = argv[1].a_w.w_float) < 0))
                    goto usage;

I think maxsize should really be a long long.

EDIT: actually, on Windows you can't even resize an array to a size larger than 2^31 because array_resize takes an int which is always 32 bit. on the other hand, 2^31 is about 13h of mono audio at 44.1 kHz...

@porres
Copy link
Contributor Author

porres commented Jun 19, 2018

I see this is related, huh? #366

@Spacechild1
Copy link
Contributor

it's related in the sense that users usually do "-maxsize HUGENUMBER" to circumvent the small default maxsize. Miller already said he is willing to remove the default value which is a relict from the old times. nevertheless, there should be a check in soundfiler_read to prevent integer overflow.

@Spacechild1
Copy link
Contributor

can be closed

@porres
Copy link
Contributor Author

porres commented Sep 4, 2018

hi, what changed?

@porres
Copy link
Contributor Author

porres commented Sep 4, 2018

I mean, I see it has a new maximum of 0x7fffffff - but can you really resize it with the -maxsize flag to something bigger than 2.14748e+09?

just wanna be sure

@Spacechild1
Copy link
Contributor

Spacechild1 commented Sep 4, 2018

but can you really resize it with the -maxsize flag to something bigger than 2.14748e+09?

no, because the point is to limit maxsize to LONG_MAX to avoid overflow (where the number wraps around). Miller has already merged my fix.
EDIT: maybe you meant if it's safe to set "-maxsize" to some very large number: so yes, because it will be clipped

@porres
Copy link
Contributor Author

porres commented Sep 4, 2018

yeah, I meant if it's safe to set "-maxsize" to some very large number bigger than 2.14748e+09 (say 4e+09).

if I get this correctly, now it's not, because it's clipped to 2.14748e+09? sorry, I don't get it... and no details have been documented about this

@Spacechild1
Copy link
Contributor

what do you mean? you can't really resize to sizes larger than LONG_MAX because Pd's internal resize function takes a long... the problem is rather that in Windows, long is 32bit, so even if you're on a 64bit Pd and you would have enough memory, you can't resize arrays larger than N > 2^31.

the problem is, garray_resize_long is a public function, and if I'm correct, changing long to long long in the signature would break ABI compatibility. one solution could be to internally have a new version which takes a long long - which gets called by both garray_resize and garray_resize_long. since the [resize( method calls the float version, we could at least make huge arrays on the patch level.

@Spacechild1
Copy link
Contributor

so again, "-maxsize HUGENUMBER" is perfectly safe now, it will simply set maxsize to the actual limit Pd can handle.

@porres
Copy link
Contributor Author

porres commented Sep 4, 2018

ok, thanks for the details, and I assume it does initialize with such actual limit it can handle, right?

@porres porres closed this as completed Sep 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants