avopt: Allow setting string/binary options to NULL

Message ID 1375267801-67865-1-git-send-email-martin@martin.st
State Deferred
Headers show

Commit Message

Martin Storsjö July 31, 2013, 10:50 a.m.
Previously you couldn't set them back to NULL, only to a zero-length
string. Now it's only disallowed to pass NULL when setting numeric
options.
---
 libavutil/opt.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Anton Khirnov July 31, 2013, 6:21 p.m. | #1
On Wed, 31 Jul 2013 13:50:01 +0300, Martin Storsjö <martin@martin.st> wrote:
> Previously you couldn't set them back to NULL, only to a zero-length
> string. Now it's only disallowed to pass NULL when setting numeric
> options.
> ---
>  libavutil/opt.c |   11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 

Does this fix some real problem?
I vaguely recall some code assuming string AVOptions will never be NULL if the
default is set.
Martin Storsjö July 31, 2013, 6:30 p.m. | #2
On Wed, 31 Jul 2013, Anton Khirnov wrote:

>
> On Wed, 31 Jul 2013 13:50:01 +0300, Martin Storsjö <martin@martin.st> wrote:
>> Previously you couldn't set them back to NULL, only to a zero-length
>> string. Now it's only disallowed to pass NULL when setting numeric
>> options.
>> ---
>>  libavutil/opt.c |   11 +++++++----
>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>
>
> Does this fix some real problem?

I kind of like to keep the distinction between NULL and an empty string in 
certain cases, but nothing that strictly requires supporting NULL right 
now.

> I vaguely recall some code assuming string AVOptions will never be NULL if the
> default is set.

Hmm, ok. If you get to looking into this area again, it'd be nice if you 
could reconfirm this then, or see if it can be allowed. I can defer the 
patch for now at least.

// Martin

Patch

diff --git a/libavutil/opt.c b/libavutil/opt.c
index f2b9473..6897a30 100644
--- a/libavutil/opt.c
+++ b/libavutil/opt.c
@@ -108,10 +108,12 @@  static int set_string_binary(void *obj, const AVOption *o, const char *val, uint
 {
     int *lendst = (int *)(dst + 1);
     uint8_t *bin, *ptr;
-    int len = strlen(val);
+    int len = val ? strlen(val) : 0;
 
     av_freep(dst);
     *lendst = 0;
+    if (!val)
+        return 0;
 
     if (len & 1)
         return AVERROR(EINVAL);
@@ -136,7 +138,8 @@  static int set_string_binary(void *obj, const AVOption *o, const char *val, uint
 static int set_string(void *obj, const AVOption *o, const char *val, uint8_t **dst)
 {
     av_freep(dst);
-    *dst = av_strdup(val);
+    if (val)
+        *dst = av_strdup(val);
     return 0;
 }
 
@@ -149,6 +152,8 @@  static int set_string(void *obj, const AVOption *o, const char *val, uint8_t **d
 static int set_string_number(void *obj, void *target_obj, const AVOption *o, const char *val, void *dst)
 {
     int ret = 0, notfirst = 0;
+    if (!val)
+        return AVERROR(EINVAL);
     for (;;) {
         int i, den = 1;
         char buf[256];
@@ -212,8 +217,6 @@  int av_opt_set(void *obj, const char *name, const char *val, int search_flags)
     const AVOption *o = av_opt_find2(obj, name, NULL, 0, search_flags, &target_obj);
     if (!o || !target_obj)
         return AVERROR_OPTION_NOT_FOUND;
-    if (!val)
-        return AVERROR(EINVAL);
 
     dst = ((uint8_t*)target_obj) + o->offset;
     switch (o->type) {