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

[Clément Bœsch]

On Sun, Oct 16, 2011 at 01:03:37AM +0200, Stefano Sabatini wrote:
> On date Saturday 2011-10-15 21:01:27 +0200, Stefano Sabatini encoded:
> > On date Saturday 2011-10-15 15:45:02 +0200, Michael Niedermayer encoded:
> > > On Sat, Oct 15, 2011 at 12:50:11PM +0200, Stefano Sabatini wrote:
> > > > 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.
> > > > 
> > > > The code is based on the OpenBSD libc strtok_r() implementation.
> > > 
> > > LGTM and great idea
> > 
> > I rewrote the function from scratch, which reads more readable to my
> > eyes (also I was missing the original copyright notice, which should
> > not be anymore required).
> > 
> > I also changed the docs, to make the doxy more compliant with the
> > official POSIX.1 strtok_r() definition, which is explicitely referenced.
> > 
> > I'll push it in a day or so if I read no more comments.
> 
> > From caee55b975bd465139e40cb76713d6cc7e65d50f 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_r()
> > 
> > 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.
> 
> 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.

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 :)

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111016/0588436c/attachment.asc>