Quoting Martin Storsjö (2016-03-19 20:01:15)
> On Sat, 19 Mar 2016, Anton Khirnov wrote:
>
> > Quoting Martin Storsjö (2016-03-18 13:01:39)
> >> From: Michael Niedermayer <michae...@gmx.at>
> >>
> >> This includes documentation and other modifications by
> >> Lukasz Marek.
> >> ---
> >> doc/APIchanges | 3 +++
> >> libavutil/opt.c | 58
> >> +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> libavutil/opt.h | 13 ++++++++++++
> >> libavutil/version.h | 2 +-
> >> 4 files changed, 75 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/doc/APIchanges b/doc/APIchanges
> >> index 20fecb9..aa6f004 100644
> >> --- a/doc/APIchanges
> >> +++ b/doc/APIchanges
> >> @@ -13,6 +13,9 @@ libavutil: 2015-08-28
> >>
> >> API changes, most recent first:
> >>
> >> +2016-xx-xx - xxxxxxx - lavu 55.7.0 - opt.h
> >> + Add av_opt_copy().
> >> +
> >> 2016-02-23 - 9200514 - lavf 57.5.0 - avformat.h
> >> Add AVStream.codecpar, deprecate AVStream.codec.
> >>
> >> diff --git a/libavutil/opt.c b/libavutil/opt.c
> >> index 5825a72..75e8970 100644
> >> --- a/libavutil/opt.c
> >> +++ b/libavutil/opt.c
> >> @@ -757,6 +757,64 @@ const AVClass *av_opt_child_class_next(const AVClass
> >> *parent, const AVClass *pre
> >> return NULL;
> >> }
> >>
> >> +static int opt_size(enum AVOptionType type)
> >> +{
> >> + switch(type) {
> >> + case AV_OPT_TYPE_INT:
> >> + case AV_OPT_TYPE_FLAGS: return sizeof(int);
> >> + case AV_OPT_TYPE_INT64: return sizeof(int64_t);
> >> + case AV_OPT_TYPE_DOUBLE: return sizeof(double);
> >> + case AV_OPT_TYPE_FLOAT: return sizeof(float);
> >> + case AV_OPT_TYPE_STRING: return sizeof(uint8_t*);
> >> + case AV_OPT_TYPE_RATIONAL: return sizeof(AVRational);
> >> + case AV_OPT_TYPE_BINARY: return sizeof(uint8_t*) + sizeof(int);
> >> + }
> >> + return 0;
> >
> > This should return an error that should be checked by the caller.
>
> Sure
>
> >> +}
> >> +
> >> +int av_opt_copy(void *dst, const void *src)
> >> +{
> >> + const AVOption *o = NULL;
> >> + const AVClass *c;
> >> + int ret = 0;
> >> +
> >> + if (!src)
> >> + return 0;
> >
> > I'm not sure about this, it looks invalid.
>
> Invalid in which sense? That this function shouldn't ever have to care
> about that case, that it should be the caller's duty to never try that?
Either that, or keep the check but return an error. But behaving as if
everything went fine does not look right to me.
>
> >> +
> >> + c = *(AVClass**)src;
> >> + if (*(AVClass**)dst && c != *(AVClass**)dst)
> >
> > Why the first check? dst is assumed to be non-NULL anyway, and if it is
> > NULL, the code lower down will explode.
>
> It's not about whether dst is null or not, but whether the AVClass pointer
> in dst has been set or not. If it hasn't been set, we don't need to see if
> it matches the source class.
Ah right, I cannot read. But then if it's NULL it continues normally?
That also seems wrong.
--
Anton Khirnov
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
