[FFmpeg-devel] [PATCH] lavu: add av_strtok_r()

[Stefano Sabatini]

On date Sunday 2011-10-16 22:40:44 +0200, Stefano Sabatini encoded:
> On date Sunday 2011-10-16 12:48:28 +0200, Stefano Sabatini encoded:
> > On date Sunday 2011-10-16 01:24:41 +0200, Clément Bœsch encoded:
> > > On Sun, Oct 16, 2011 at 01:03:37AM +0200, Stefano Sabatini wrote:
> > [...]
> > > > Bikeshed time!
> > > > 
> > > > What do people prefer between av_strtok (shorter, more consistent with
> > > > ffmpeg naming scheme, but confusing as strtok() is a function with a
> > > > different semantics), and av_strtok_r (more similar to the POSIX.1
> > > > function from which borrows its semantics)?
> > > > 
> > > 
> > > I like av_strtok(), but I'm fine with av_strtok_r().
> > > 
> > > > My guts tell me that av_strtok_r() will cause less overall confusion,
> > > > so I'd stick with that if I read no arguments in favor of av_strtok().
> > > 
> > > If the Doxy is clear enough it should do the trick ("this function is
> > > similar to strtok_r() and is thread safe"); it will be read anyway ("oh, a
> > > string token function in ffmpeg, why the reimplementation? let's read
> > > it"), and if not it will just be copy/paste from existing code without
> > > caring much about the details.
> > > 
> > > Also keep in mind this function is generally inlined in already pretty
> > > long lines, thus keeping the simple naming scheme is a plus.
> > 
> > Your arguments are fine, but I also considered that we may need not to
> > be too strict about the strtok_r() compliance.
> > 
> > Indeed my new definition differs in some details, so it is better to
> > give it a different name.
> > 
> > The extension regards the value assigned to saveptr after each call,
> > which is not defined by POSIX.1. I'm specifying it in the doxy, which
> > doesn't affect strtok_r() compatibility at all, but which allows to do
> > things like:
> > 
> >     w_name = av_strtok(print_format, "=", &buf);
> >     w_args = buf;
> > 
> > which would be wrong with strtok_r() (the third strtok_r() argument is
> > defined as an opaque pointer, and the specifics tells nothing about
> > what it contains).
> > 
> > > Anyway, I think I'll reorder my priorities now since this bikeshed mail is
> > > the bigger contribution I made these last days… So feel free to ignore it
> > > of course :)
> > 
> > No, it helped to spot some problems which I didn't consider in my
> > first patch, so it was somehow useful...
> 
> > From 231390ea7e297776979941eb21f9d65cea1e5379 Mon Sep 17 00:00:00 2001
> > From: Stefano Sabatini <stefasab at gmail.com>
> > Date: Sat, 15 Oct 2011 00:14:37 +0200
> > Subject: [PATCH] lavu: add av_strtok()
> > 
> > The function strtok_r() is part of the POSIX.1 specification, but is not
> > available on some platforms. We provide an internal implementation, so we
> > do not need to rely on a platform implementation.
> 
> I'll apply in one or two days if no one has more comments.

Pushed (but missed the minor update due to some mess I did with git,
committed separately).