[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