Christophergoing slightly mad. Old code is fine, new is borked. These buffers are never freed (just left for OS to clean up when program exits).
anyone?
Wait, `calloc((MAX_INPUTS + 1), UDP_PER_RF_PER_SUB * sizeof(char *));` works?
But the borked one only multiplies 545 by 5002 to get 2726090 items?
neoluddite@shrydar probably not related, but are you sure you want to * by MAX_INPUTS + 1 in the calloc in the new version? That looks like an off by one error to me, probably carried over from the for loop where you need it.
Would possibly help if you defined "borked" as well ;)
@shrydar you may also be allocating one more input than you need with the for loops? MAX_INPUTS = 3 will cycle through input = 0, 1, 2, 3 - which is 4 inputs
@neoluddite There’s a historical reason for that. Indices are 1-based, so I will be dropping the first allocation at some point
Bakunin Boys@shrydar does calloc take a short or something?
zirias (on snac)Borked in what way? What's the declared type of
.udp_volts?@zirias a long way down the line a map of which pointers were never initialised to point to data has a mismatch with data output by concatenating the data pointed to with runs of zeros for null pointers. Changing the params to calloc seems to fix it (cf other reply in thread), which makes no sense at all. Weekend time now I think.
`.udp_volts` is a char***
(a few lines earlier there's a `sub[slot].udp_volts = calloc(MAX_INPUTS + 1, sizeof(char **));` )
Ok, I can't immediately spot the issue with the parts of the code seen so far. If I had to debug this, I'd fire some tooling on it (e.g. the compiler's "address sanitizer" and similar, valgrind, ...) first.
But then, this also looks like some code in desperate need of refactoring. "Three stars" should almost always raise an eyebrow: https://wiki.c2.com/?ThreeStarProgrammer ... and the
I'd also suggest to use expressions instead of type names with the
But then, this also looks like some code in desperate need of refactoring. "Three stars" should almost always raise an eyebrow: https://wiki.c2.com/?ThreeStarProgrammer ... and the
MAX_INPUTS constant looks weird, if it's actually the maximum index, making it one less than the number of inputs 🧐I'd also suggest to use expressions instead of type names with the
sizeof operator, making the code more robust to change, e.g. sizeof(char **) → sizeof *sub[slot].udp_volts in the above line of code, but that's a stylistic thing...@zirias I mean, no arguments about the need for refactoring. I inherited a 3000 line monolith of very old-school C that nonetheless mostly gets the job done. I’m gradually refactoring and modernising while I make it a bit more robust and efficient and add a few new features along the way.
(and yes, inputs are 1-based because there’s a mapping elsewhere that uses ‘0’ for “as yet unassigned”)
Not A Dropbear@shrydar let me make a couple of wild unsubstantiated guesses.
If UDP_PER_RF_PER_SUB isn’t a multiple of some magic number of bytes (for example, if it isn’t a multiple of 4, or whatever) maybe the old code is effectively being given an extra byte or two or three than what it’s asking for. If there’s a small buffer overrun, that might then be masked, because each separate call is returning a chunk of memory that is word-aligned or whatever. This is predicated on the code thinking in bytes but the allocator and memory protection thinking in words or whatever, which I don’t know if that’s what’s really happening or not.
The second guess: again, an overrun. If those buffer are read/written in a thread, and a buffer overrun occurs, and the thread gets killed for overrunning but the app isn’t noticing, and it’s happening e.g. during the code in which the thread would be exiting anyway, so the developers and users don’t notice either, then the new code would behave differently because the overrun now smashes the next array entry.