Re: [PATCH 1/2] libmpdemux/mf: Replace sprintf by mp_asprintf

On 2021-04-28 21:09 +0200, Reimar Döffinger wrote:
> > @@ -142,19 +143,23 @@ mf_t* open_mf(char * filename){
> >
> >  while ( error_count < 5 )
> >   {
> > -   sprintf( fname,filename,count++ );
> > +   fname = mp_asprintf( filename,count++ );
>
> A new variable local to this block would make it much easier to argue memory correctness.
> This probably also applies to the other usages of this “fname” variable,
> but that’s a somewhat separate issue.

Well. Not sure I fully grasp how you reason about it.

There's always a "goto exit_mf", which looks like this:
    exit_mf:
     free( fname );
     return mf;

We definitely don't want that to free a needed path and
that's why we need fname to be NULL after we went into
the printf format loop.

The other gotcha is: We detect the end of the sequence
by running into errors. Those failed tries should be
freed. Maybe my patch could be simplified further by
exploiting that, but as it's non-obvious and might make
future errors more likely I don't really want to do
that.

Did I overlook another problem?

If you want to review it, I could try to refactor the
individual ways to gather the file lists into individual
functions. Localizing variables to those functions as
much as possible.

Shall it be OK to commit this patch anyway?


  Alexander
_______________________________________________
MPlayer-dev-eng mailing list
[email protected]
https://lists.mplayerhq.hu/mailman/listinfo/mplayer-dev-eng