diff mbox series

[FFmpeg-devel,1/3] avutil/dict: av_realloc -> av_realloc_array()

Message ID 1590845462-9163-1-git-send-email-lance.lmwang@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel,1/3] avutil/dict: av_realloc -> av_realloc_array() | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Lance Wang May 30, 2020, 1:31 p.m. UTC
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavutil/dict.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Nicolas George May 30, 2020, 1:32 p.m. UTC | #1
lance.lmwang@gmail.com (12020-05-30):
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavutil/dict.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavutil/dict.c b/libavutil/dict.c
> index 0ea7138..a1107b1 100644
> --- a/libavutil/dict.c
> +++ b/libavutil/dict.c
> @@ -103,8 +103,8 @@ int av_dict_set(AVDictionary **pm, const char *key, const char *value,
>          av_free(tag->key);
>          *tag = m->elems[--m->count];
>      } else if (copy_value) {
> -        AVDictionaryEntry *tmp = av_realloc(m->elems,
> -                                            (m->count + 1) * sizeof(*m->elems));

> +        AVDictionaryEntry *tmp = av_realloc_array(m->elems,
> +                                            m->count + 1, sizeof(*m->elems));

If you change something, make sure the alignment is correct.

>          if (!tmp)
>              goto err_out;
>          m->elems = tmp;

Regards,
Lance Wang May 30, 2020, 1:55 p.m. UTC | #2
On Sat, May 30, 2020 at 03:32:18PM +0200, Nicolas George wrote:
> lance.lmwang@gmail.com (12020-05-30):
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavutil/dict.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavutil/dict.c b/libavutil/dict.c
> > index 0ea7138..a1107b1 100644
> > --- a/libavutil/dict.c
> > +++ b/libavutil/dict.c
> > @@ -103,8 +103,8 @@ int av_dict_set(AVDictionary **pm, const char *key, const char *value,
> >          av_free(tag->key);
> >          *tag = m->elems[--m->count];
> >      } else if (copy_value) {
> > -        AVDictionaryEntry *tmp = av_realloc(m->elems,
> > -                                            (m->count + 1) * sizeof(*m->elems));
> 
> > +        AVDictionaryEntry *tmp = av_realloc_array(m->elems,
> > +                                            m->count + 1, sizeof(*m->elems));
> 
> If you change something, make sure the alignment is correct.
I think it's cosmetic changes, so I didn't change the alignment for the first version.
If it's OK, I'll update with format changes also.

> 
> >          if (!tmp)
> >              goto err_out;
> >          m->elems = tmp;
> 
> Regards,
> 
> -- 
>   Nicolas George
Nicolas George May 30, 2020, 2:03 p.m. UTC | #3
lance.lmwang@gmail.com (12020-05-30):
> I think it's cosmetic changes, so I didn't change the alignment for the first version.
> If it's OK, I'll update with format changes also.

Please, try to see the moon, not the finger.

Why do you think we insist on cosmetic changes on separate patches?
Because they make reviewing the actual changes harder.

But remember, changes in patches are shown lines per line.

Regards,
James Almer May 30, 2020, 2:14 p.m. UTC | #4
On 5/30/2020 10:55 AM, lance.lmwang@gmail.com wrote:
> On Sat, May 30, 2020 at 03:32:18PM +0200, Nicolas George wrote:
>> lance.lmwang@gmail.com (12020-05-30):
>>> From: Limin Wang <lance.lmwang@gmail.com>
>>>
>>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
>>> ---
>>>  libavutil/dict.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavutil/dict.c b/libavutil/dict.c
>>> index 0ea7138..a1107b1 100644
>>> --- a/libavutil/dict.c
>>> +++ b/libavutil/dict.c
>>> @@ -103,8 +103,8 @@ int av_dict_set(AVDictionary **pm, const char *key, const char *value,
>>>          av_free(tag->key);
>>>          *tag = m->elems[--m->count];
>>>      } else if (copy_value) {
>>> -        AVDictionaryEntry *tmp = av_realloc(m->elems,
>>> -                                            (m->count + 1) * sizeof(*m->elems));
>>
>>> +        AVDictionaryEntry *tmp = av_realloc_array(m->elems,
>>> +                                            m->count + 1, sizeof(*m->elems));
>>
>> If you change something, make sure the alignment is correct.
> I think it's cosmetic changes, so I didn't change the alignment for the first version.
> If it's OK, I'll update with format changes also.

It makes sense to not bother with cosmetics when it reduces the amount
of changed lines and a patch about functional changes, but in this case
the line in question is going to be changed no matter what, so might as
well just align it at the same time and save yourself an extra commit
for it.

> 
>>
>>>          if (!tmp)
>>>              goto err_out;
>>>          m->elems = tmp;
>>
>> Regards,
>>
>> -- 
>>   Nicolas George
> 
> 
>
Lance Wang May 30, 2020, 2:14 p.m. UTC | #5
On Sat, May 30, 2020 at 04:03:33PM +0200, Nicolas George wrote:
> lance.lmwang@gmail.com (12020-05-30):
> > I think it's cosmetic changes, so I didn't change the alignment for the first version.
> > If it's OK, I'll update with format changes also.
> 
> Please, try to see the moon, not the finger.
> 
> Why do you think we insist on cosmetic changes on separate patches?
> Because they make reviewing the actual changes harder.
> 
> But remember, changes in patches are shown lines per line.

OK, have update and fixed the alignment also.

> 
> Regards,
> 
> -- 
>   Nicolas George



> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Lance Wang June 1, 2020, 2:05 p.m. UTC | #6
On Sat, May 30, 2020 at 11:14:03AM -0300, James Almer wrote:
> On 5/30/2020 10:55 AM, lance.lmwang@gmail.com wrote:
> > On Sat, May 30, 2020 at 03:32:18PM +0200, Nicolas George wrote:
> >> lance.lmwang@gmail.com (12020-05-30):
> >>> From: Limin Wang <lance.lmwang@gmail.com>
> >>>
> >>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> >>> ---
> >>>  libavutil/dict.c | 4 ++--
> >>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/libavutil/dict.c b/libavutil/dict.c
> >>> index 0ea7138..a1107b1 100644
> >>> --- a/libavutil/dict.c
> >>> +++ b/libavutil/dict.c
> >>> @@ -103,8 +103,8 @@ int av_dict_set(AVDictionary **pm, const char *key, const char *value,
> >>>          av_free(tag->key);
> >>>          *tag = m->elems[--m->count];
> >>>      } else if (copy_value) {
> >>> -        AVDictionaryEntry *tmp = av_realloc(m->elems,
> >>> -                                            (m->count + 1) * sizeof(*m->elems));
> >>
> >>> +        AVDictionaryEntry *tmp = av_realloc_array(m->elems,
> >>> +                                            m->count + 1, sizeof(*m->elems));
> >>
> >> If you change something, make sure the alignment is correct.
> > I think it's cosmetic changes, so I didn't change the alignment for the first version.
> > If it's OK, I'll update with format changes also.
> 
> It makes sense to not bother with cosmetics when it reduces the amount
> of changed lines and a patch about functional changes, but in this case
> the line in question is going to be changed no matter what, so might as
> well just align it at the same time and save yourself an extra commit
> for it.

thanks, will follow it next time.

> 
> > 
> >>
> >>>          if (!tmp)
> >>>              goto err_out;
> >>>          m->elems = tmp;
> >>
> >> Regards,
> >>
> >> -- 
> >>   Nicolas George
> > 
> > 
> > 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavutil/dict.c b/libavutil/dict.c
index 0ea7138..a1107b1 100644
--- a/libavutil/dict.c
+++ b/libavutil/dict.c
@@ -103,8 +103,8 @@  int av_dict_set(AVDictionary **pm, const char *key, const char *value,
         av_free(tag->key);
         *tag = m->elems[--m->count];
     } else if (copy_value) {
-        AVDictionaryEntry *tmp = av_realloc(m->elems,
-                                            (m->count + 1) * sizeof(*m->elems));
+        AVDictionaryEntry *tmp = av_realloc_array(m->elems,
+                                            m->count + 1, sizeof(*m->elems));
         if (!tmp)
             goto err_out;
         m->elems = tmp;