[FFmpeg-cvslog] r25165 - in trunk: Changelog configure doc/filters.texi libavfilter/Makefile libavfilter/allfilters.c libavfilter/avfilter.h libavfilter/vf_frei0r.c

[Måns Rullgård]

Stefano Sabatini <stefano.sabatini-lala at poste.it> writes:

>> >> > @@ -1050,6 +1052,7 @@ HAVE_LIST="
>> >> >      poll_h
>> >> >      setrlimit
>> >> >      strerror_r
>> >> > +    strtok_r
>> >> >      struct_addrinfo
>> >> >      struct_ipv6_mreq
>> >> >      struct_sockaddr_in6
>> >> 
>> >> I told you not to check for strtok_r.  It exists on all POSIX systems,
>> >> and frei0r doesn't work anywhere else (dlopen).  Please remove this
>> >> pointless check, and also please respect the review comments in future.
>> >
>> > POSIX => dlopen
>> > but not:
>> > dlopen => POSIX
>> >
>> > so if dlopen is present you can't infer that strtok_r exists.
>> >
>> > And note that we already have a check for strerror_r, which is
>> > supported by POSIX but is not supported in MinGW, following the same
>> > logic it makes sense to keep the check for strtok_r.
>> 
>> We check for strerror_r because it causes an actual build failure if
>> we do not.  That is not the case for strtok_r.  Gratuitous checks only
>> make configure run slower and bloat the build system for no gain.
>> 
>> For just about any function we use, no matter how standard, there
>> surely exists some system, somewhere that doesn't have it.
>
> Check attached.

Just to be sure, does frei0r work on Windows?

>> >> You have changed this substantially from what was reviewed, and not in
>> >> the way I requested.  What is the point of reviews if you are going to
>> >> ignore them?
>> >
>> > Basically I rewrote it to be more strict with the frei0r specs.
>> 
>> Without bothering to respond to my comments, and without posting it
>> for review.
>
> I believed I addressed to your comments with that change, 

You did not.

> and the patch was posted since more than a week (09-13).

Maybe I missed it.  You could have reminded me.  I've seen you on IRC
several times since then.

-- 
M?ns Rullg?rd
mans at mansr.com