Re: [FFmpeg-devel] [PATCH 1/2] lavu/dict: fix set function when reuse existing key pointer

2015-04-02 Thread Lukasz Marek 

On 02.04.2015 01:09, Michael Niedermayer wrote:

On Thu, Apr 02, 2015 at 12:37:34AM +0200, Lukasz Marek wrote:

On 01.04.2015 04:33, Michael Niedermayer wrote:

On Wed, Apr 01, 2015 at 03:25:23AM +0200, Lukasz Marek wrote:

Fixes following scenario:

av_dict_set(&d, "key", "old", 0);
AVDictionaryEentry *e = av_dict_get(d, "key", NULL, 0);
av_dict_set(&d, e->key, "new", 0);

Signed-off-by: Lukasz Marek 
---
  libavutil/dict.c | 18 --
  1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/libavutil/dict.c b/libavutil/dict.c
index 0d54c79..3163894 100644
--- a/libavutil/dict.c
+++ b/libavutil/dict.c
@@ -72,6 +72,7 @@ int av_dict_set(AVDictionary **pm, const char *key, const 
char *value,
  AVDictionary *m = *pm;
  AVDictionaryEntry *tag = av_dict_get(m, key, NULL, flags);
  char *oldval = NULL;
+int the_same_key = 0;



does it work to av_strdup() both key and value if they need to be
strduped but at the top of the function and simplify the rest
accordingly ?



something like attached?


yes, LGTM if tested with valgrind or similar


Yes, I've tested with valgrind. Pushed.

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] lavu/dict: fix set function when reuse existing key pointer

2015-04-01 Thread Michael Niedermayer 
On Thu, Apr 02, 2015 at 12:37:34AM +0200, Lukasz Marek wrote:
> On 01.04.2015 04:33, Michael Niedermayer wrote:
> >On Wed, Apr 01, 2015 at 03:25:23AM +0200, Lukasz Marek wrote:
> >>Fixes following scenario:
> >>
> >>av_dict_set(&d, "key", "old", 0);
> >>AVDictionaryEentry *e = av_dict_get(d, "key", NULL, 0);
> >>av_dict_set(&d, e->key, "new", 0);
> >>
> >>Signed-off-by: Lukasz Marek 
> >>---
> >>  libavutil/dict.c | 18 --
> >>  1 file changed, 16 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/libavutil/dict.c b/libavutil/dict.c
> >>index 0d54c79..3163894 100644
> >>--- a/libavutil/dict.c
> >>+++ b/libavutil/dict.c
> >>@@ -72,6 +72,7 @@ int av_dict_set(AVDictionary **pm, const char *key, const 
> >>char *value,
> >>  AVDictionary *m = *pm;
> >>  AVDictionaryEntry *tag = av_dict_get(m, key, NULL, flags);
> >>  char *oldval = NULL;
> >>+int the_same_key = 0;
> >>
> >
> >does it work to av_strdup() both key and value if they need to be
> >strduped but at the top of the function and simplify the rest
> >accordingly ?
> 
> 
> something like attached?

yes, LGTM if tested with valgrind or similar

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato 


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] lavu/dict: fix set function when reuse existing key pointer

2015-04-01 Thread Lukasz Marek 

On 01.04.2015 04:33, Michael Niedermayer wrote:

On Wed, Apr 01, 2015 at 03:25:23AM +0200, Lukasz Marek wrote:

Fixes following scenario:

av_dict_set(&d, "key", "old", 0);
AVDictionaryEentry *e = av_dict_get(d, "key", NULL, 0);
av_dict_set(&d, e->key, "new", 0);

Signed-off-by: Lukasz Marek 
---
  libavutil/dict.c | 18 --
  1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/libavutil/dict.c b/libavutil/dict.c
index 0d54c79..3163894 100644
--- a/libavutil/dict.c
+++ b/libavutil/dict.c
@@ -72,6 +72,7 @@ int av_dict_set(AVDictionary **pm, const char *key, const 
char *value,
  AVDictionary *m = *pm;
  AVDictionaryEntry *tag = av_dict_get(m, key, NULL, flags);
  char *oldval = NULL;
+int the_same_key = 0;



does it work to av_strdup() both key and value if they need to be
strduped but at the top of the function and simplify the rest
accordingly ?



something like attached?

>From 2a36567b12ae14798306a988061fe30474c5c332 Mon Sep 17 00:00:00 2001
From: Lukasz Marek 
Date: Wed, 1 Apr 2015 20:01:30 +0200
Subject: [PATCH] lavu/dict: fix set function when reuse existing key pointer

Fixes following scenario:

av_dict_set(&d, "key", "old", 0);
AVDictionaryEentry *e = av_dict_get(d, "key", NULL, 0);
av_dict_set(&d, e->key, "new", 0);

Signed-off-by: Lukasz Marek 
---
 libavutil/dict.c | 44 
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/libavutil/dict.c b/libavutil/dict.c
index 0d54c79..1dfcb68 100644
--- a/libavutil/dict.c
+++ b/libavutil/dict.c
@@ -71,17 +71,25 @@ int av_dict_set(AVDictionary **pm, const char *key, const char *value,
 {
 AVDictionary *m = *pm;
 AVDictionaryEntry *tag = av_dict_get(m, key, NULL, flags);
-char *oldval = NULL;
+char *oldval = NULL, *copy_key = NULL, *copy_value = NULL;
 
+if (flags & AV_DICT_DONT_STRDUP_KEY)
+copy_key = (void *)key;
+else
+copy_key = av_strdup(key);
+if (flags & AV_DICT_DONT_STRDUP_VAL)
+copy_value = (void *)value;
+else if (copy_key)
+copy_value = av_strdup(value);
 if (!m)
 m = *pm = av_mallocz(sizeof(*m));
-if (!m)
+if (!m || (key && !copy_key) || (value && !copy_value))
 goto err_out;
 
 if (tag) {
 if (flags & AV_DICT_DONT_OVERWRITE) {
-if (flags & AV_DICT_DONT_STRDUP_KEY) av_free((void*)key);
-if (flags & AV_DICT_DONT_STRDUP_VAL) av_free((void*)value);
+av_free(copy_key);
+av_free(copy_value);
 return 0;
 }
 if (flags & AV_DICT_APPEND)
@@ -97,27 +105,23 @@ int av_dict_set(AVDictionary **pm, const char *key, const char *value,
 goto err_out;
 m->elems = tmp;
 }
-if (value) {
-if (flags & AV_DICT_DONT_STRDUP_KEY)
-m->elems[m->count].key = (char*)(intptr_t)key;
-else
-m->elems[m->count].key = av_strdup(key);
-if (!m->elems[m->count].key)
-goto err_out;
-if (flags & AV_DICT_DONT_STRDUP_VAL) {
-m->elems[m->count].value = (char*)(intptr_t)value;
-} else if (oldval && flags & AV_DICT_APPEND) {
-int len = strlen(oldval) + strlen(value) + 1;
+if (copy_value) {
+m->elems[m->count].key = copy_key;
+m->elems[m->count].value = copy_value;
+if (oldval && flags & AV_DICT_APPEND) {
+int len = strlen(oldval) + strlen(copy_value) + 1;
 char *newval = av_mallocz(len);
 if (!newval)
 goto err_out;
 av_strlcat(newval, oldval, len);
 av_freep(&oldval);
-av_strlcat(newval, value, len);
+av_strlcat(newval, copy_value, len);
 m->elems[m->count].value = newval;
-} else
-m->elems[m->count].value = av_strdup(value);
+av_freep(©_value);
+}
 m->count++;
+} else {
+av_freep(©_key);
 }
 if (!m->count) {
 av_freep(&m->elems);
@@ -131,8 +135,8 @@ err_out:
 av_freep(&m->elems);
 av_freep(pm);
 }
-if (flags & AV_DICT_DONT_STRDUP_KEY) av_free((void*)key);
-if (flags & AV_DICT_DONT_STRDUP_VAL) av_free((void*)value);
+av_free(copy_key);
+av_free(copy_value);
 return AVERROR(ENOMEM);
 }
 
-- 
1.9.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] lavu/dict: fix set function when reuse existing key pointer

2015-03-31 Thread Michael Niedermayer 
On Wed, Apr 01, 2015 at 03:25:23AM +0200, Lukasz Marek wrote:
> Fixes following scenario:
> 
> av_dict_set(&d, "key", "old", 0);
> AVDictionaryEentry *e = av_dict_get(d, "key", NULL, 0);
> av_dict_set(&d, e->key, "new", 0);
> 
> Signed-off-by: Lukasz Marek 
> ---
>  libavutil/dict.c | 18 --
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/libavutil/dict.c b/libavutil/dict.c
> index 0d54c79..3163894 100644
> --- a/libavutil/dict.c
> +++ b/libavutil/dict.c
> @@ -72,6 +72,7 @@ int av_dict_set(AVDictionary **pm, const char *key, const 
> char *value,
>  AVDictionary *m = *pm;
>  AVDictionaryEntry *tag = av_dict_get(m, key, NULL, flags);
>  char *oldval = NULL;
> +int the_same_key = 0;
>  

does it work to av_strdup() both key and value if they need to be
strduped but at the top of the function and simplify the rest
accordingly ?

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 3
"Rare item" - "Common item with rare defect or maybe just a lie"
"Professional" - "'Toy' made in china, not functional except as doorstop"
"Experts will know" - "The seller hopes you are not an expert"


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 1/2] lavu/dict: fix set function when reuse existing key pointer

2015-03-31 Thread Lukasz Marek 
Fixes following scenario:

av_dict_set(&d, "key", "old", 0);
AVDictionaryEentry *e = av_dict_get(d, "key", NULL, 0);
av_dict_set(&d, e->key, "new", 0);

Signed-off-by: Lukasz Marek 
---
 libavutil/dict.c | 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/libavutil/dict.c b/libavutil/dict.c
index 0d54c79..3163894 100644
--- a/libavutil/dict.c
+++ b/libavutil/dict.c
@@ -72,6 +72,7 @@ int av_dict_set(AVDictionary **pm, const char *key, const 
char *value,
 AVDictionary *m = *pm;
 AVDictionaryEntry *tag = av_dict_get(m, key, NULL, flags);
 char *oldval = NULL;
+int the_same_key = 0;
 
 if (!m)
 m = *pm = av_mallocz(sizeof(*m));
@@ -88,7 +89,10 @@ int av_dict_set(AVDictionary **pm, const char *key, const 
char *value,
 oldval = tag->value;
 else
 av_free(tag->value);
-av_free(tag->key);
+if (tag->key != key)
+av_free(tag->key);
+else
+the_same_key = 1;
 *tag = m->elems[--m->count];
 } else {
 AVDictionaryEntry *tmp = av_realloc(m->elems,
@@ -98,7 +102,7 @@ int av_dict_set(AVDictionary **pm, const char *key, const 
char *value,
 m->elems = tmp;
 }
 if (value) {
-if (flags & AV_DICT_DONT_STRDUP_KEY)
+if (flags & AV_DICT_DONT_STRDUP_KEY || the_same_key)
 m->elems[m->count].key = (char*)(intptr_t)key;
 else
 m->elems[m->count].key = av_strdup(key);
@@ -271,6 +275,7 @@ static void test_separators(const AVDictionary *m, const 
char pair, const char v
 int main(void)
 {
 AVDictionary *dict = NULL;
+AVDictionaryEntry *e;
 char *buffer = NULL;
 
 printf("Testing av_dict_get_string() and av_dict_parse_string()\n");
@@ -298,6 +303,15 @@ int main(void)
 test_separators(dict, '"', '\'');
 av_dict_free(&dict);
 
+//valgrind sensible test
+printf("\nTesting av_dict_set() with existing AVDictionaryEntry.key as 
key\n");
+av_dict_set(&dict, "key", "old", 0);
+e = av_dict_get(dict, "key", NULL, 0);
+av_dict_set(&dict, e->key, "new val OK", 0);
+e = av_dict_get(dict, "key", NULL, 0);
+printf("%s\n", e->value);
+av_dict_free(&dict);
+
 return 0;
 }
 #endif
-- 
1.9.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel