[vlc-devel] [PATCH] modules: use 16-bit ints for option counts

Lyndon Brown jnqnfe at gmail.com
Wed Sep 30 19:05:51 CEST 2020


I can understand and appreciate that that makes things a little more
simple in such ways. I think we can agree that we've spent far too much
time already debating the size change aspect. It's just not worth it,
let's just forget it.

As for the enhancement of adding a limit to the number of items a
plugin can generate before we start rejecting them and giving an error,
I just remain a little uncertain about your position. On the one hand
you argue that you don't want such a new untested error path, despite
it being so trivial to understand, and yet on the other you were
arguing that it's so trivial for a situation to occur with a plugin
that could cause a runaway generation of items. Allowing such a
situation to grab as much memory as possible and thus have VLC bog down
the system until it's killed isn't ideal; a limiter would stop that
happening, in the unlikely event that it occurred. We have existing
limiters to cap things. Do you agree or disagree on adding such a
limiter? (if *not* done with an assert).


On Wed, 2020-09-30 at 11:52 +0300, Rémi Denis-Courmont wrote:
> Hi,
> 
> When I manipulate the byte size of an in -memory object, or the
> element count of an in-memory array, I use size_t. I know then that I
> don't have to worry about overflow, that a reference will be size_t*,
> that the format string modifier is z.
> 
> I have enough other potential problems to worry about when reading or
> writing code.
> 
> Le 29 septembre 2020 22:09:48 GMT+03:00, Lyndon Brown <
> jnqnfe at gmail.com> a écrit :
> > On Tue, 2020-09-29 at 18:09 +0300, Rémi Denis-Courmont wrote:
> > > Le tiistaina 29. syyskuuta 2020, 2.56.18 EEST Lyndon Brown a
> > > écrit :
> > > > > Which is exactly the current situation. With size_t, you
> > > > > can't
> > > > > overflow.
> > > > Who says I'd not tested it?
> > > Most error paths are not tested, or only tested once when the
> > > code is
> > > first 
> > > written. I'm not interesting in arguing about this broadly
> > > accepted
> > > notion.
> > 
> > Granted.
> > 
> > But both changes, the size change and catching a runaway series of
> > creation requests, whichever or both you're referring to, are
> > rather
> > trivial to understand and follow, to be confident in. I don't think
> > such concern is warranted here.
> > 
> > I spent a lot of time exploring the option and plugin descriptor
> > handling areas of the codebase both in terms of the Rust conversion
> > project and in terms of a bunch of work pending submission (per
> > stuff
> > beneath it in the big patch tree gradually getting processed). This
> > isn't just a random tweak without understanding of how this stuff
> > is
> > interacted with throughout the plugin descriptor and option
> > handling
> > code, I've looked and understood and carefully made changes.
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel



More information about the vlc-devel mailing list