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

[Stefano Sabatini]

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.