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

[Stefano Sabatini]

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...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-lavu-add-av_strtok.patch
Type: text/x-diff
Size: 8105 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111016/482dc2c1/attachment.bin>