[vlc-devel] [PATCH 09/12] network: io: Remove VLA usages
Rémi Denis-Courmont
remi at remlab.net
Thu Dec 10 16:38:23 CET 2020
Le jeudi 10 décembre 2020, 09:05:07 EET Steve Lhomme a écrit :
> In this case the documentation doesn't mention it cannot fail with
> ENOMEM which is in line with the code, which can fail with ENOMEM as
> proved earlier.
I don't think you can really compare ENOMEM from within the system call to
ENOMEM from the user heap. But even for the sake of the argument, i don't
think an existing bug is an excuse for adding another similar bug.
And there's also the performance aspect here. Variable-length arrays are
slower than fixed-length arrays in small part by necessity, and in large part
because GCC adn LLVM implementations suck at optimising VLAs. But they are
still far faster (and more constant) than heap.
> You're just inventing new rules that exist neither in
> the code nor in the documentation.
And you're just switching side as is convenient. Not so long ago, you refused
to use standard *nonoptional* C11 heap management functions. And somehow now
you're all about using standard C11.
It's quite tiring that when it came to C11 vs nonstandard heap or macros, I
was more or less the only one to defend the standard. And now suddenly,
everybody seems to be super-fond of C11 and arguing.
> > No. We've been through this many times. Event handling cannot fail. If it
> > does, depending on the case, it's either a live loop, a dead lock, both at
> > the same time (a.k.a. an infinite busy loop).
>
> Are you suggesting net_Accept should not have any return value and
> errors be handled internally ?
No. I'm saying that there are essentially three cases right now: succesful
return, error return where the right thing to do is to retry (EINTR, kernel
ENOMEM, dequeing an ICMP error), and error returns that point to a bug in the
caller (EBADF, EINVAL).
There are currently no cases where the right thing for a non-buggy caller to
do would not be to either retry or reiterate the outer loop. But with this
code, you have a new error, that I don't even know how to handle, TBH. Busy
loop is clearly not the right approach.
--
Rémi Denis-Courmont
More information about the vlc-devel
mailing list