[FFmpeg-devel] lavu/mem: un-inline av_size_mult()

There seems to be no compelling reason for it to be inline.
---
 libavutil/mem.c | 18 ++++++++++++++++++
 libavutil/mem.h | 19 +------------------
 2 files changed, 19 insertions(+), 18 deletions(-)

On 5/31/2021 6:26 AM, Anton Khirnov wrote:
> There seems to be no compelling reason for it to be inline.
> ---
>   libavutil/mem.c | 18 ++++++++++++++++++
>   libavutil/mem.h | 19 +------------------
>   2 files changed, 19 insertions(+), 18 deletions(-)
> 
> diff --git a/libavutil/mem.c b/libavutil/mem.c
> index a52d33d4a6..063635fb22 100644
> --- a/libavutil/mem.c
> +++ b/libavutil/mem.c
> @@ -547,3 +547,21 @@ void av_fast_mallocz(void *ptr, unsigned int *size, size_t min_size)
>   {
>       fast_malloc(ptr, size, min_size, 1);
>   }
> +
> +int av_size_mult(size_t a, size_t b, size_t *r)
> +{
> +    size_t t;
> +
> +#if (!defined(__INTEL_COMPILER) && AV_GCC_VERSION_AT_LEAST(5,1)) || AV_HAS_BUILTIN(__builtin_mul_overflow)
> +    if (__builtin_mul_overflow(a, b, &t))
> +        return AVERROR(EINVAL);
> +#else
> +    t = a * b;
> +    /* Hack inspired from glibc: don't try the division if nelem and elsize
> +     * are both less than sqrt(SIZE_MAX). */
> +    if ((a | b) >= ((size_t)1 << (sizeof(size_t) * 4)) && a && t / a != b)
> +        return AVERROR(EINVAL);
> +#endif
> +    *r = t;
> +    return 0;
> +}
> diff --git a/libavutil/mem.h b/libavutil/mem.h
> index c876111afb..e9d343eaf0 100644
> --- a/libavutil/mem.h
> +++ b/libavutil/mem.h
> @@ -31,7 +31,6 @@
>   #include <stdint.h>
>   
>   #include "attributes.h"
> -#include "error.h"
>   #include "avutil.h"
>   #include "version.h"
>   
> @@ -672,23 +671,7 @@ void *av_dynarray2_add(void **tab_ptr, int *nb_ptr, size_t elem_size,
>    * @param[out] r   Pointer to the result of the operation
>    * @return 0 on success, AVERROR(EINVAL) on overflow
>    */

You should also move the doxy.

LGTM otherwise.

> -static inline int av_size_mult(size_t a, size_t b, size_t *r)
> -{
> -    size_t t;
> -
> -#if (!defined(__INTEL_COMPILER) && AV_GCC_VERSION_AT_LEAST(5,1)) || AV_HAS_BUILTIN(__builtin_mul_overflow)
> -    if (__builtin_mul_overflow(a, b, &t))
> -        return AVERROR(EINVAL);
> -#else
> -    t = a * b;
> -    /* Hack inspired from glibc: don't try the division if nelem and elsize
> -     * are both less than sqrt(SIZE_MAX). */
> -    if ((a | b) >= ((size_t)1 << (sizeof(size_t) * 4)) && a && t / a != b)
> -        return AVERROR(EINVAL);
> -#endif
> -    *r = t;
> -    return 0;
> -}
> +int av_size_mult(size_t a, size_t b, size_t *r);
>   
>   /**
>    * Set the maximum size that may be allocated in one block.
>

Quoting James Almer (2021-05-31 13:56:20)
> On 5/31/2021 6:26 AM, Anton Khirnov wrote:
> > There seems to be no compelling reason for it to be inline.
> > ---
> >   libavutil/mem.c | 18 ++++++++++++++++++
> >   libavutil/mem.h | 19 +------------------
> >   2 files changed, 19 insertions(+), 18 deletions(-)
> > 
> > diff --git a/libavutil/mem.c b/libavutil/mem.c
> > index a52d33d4a6..063635fb22 100644
> > --- a/libavutil/mem.c
> > +++ b/libavutil/mem.c
> > @@ -547,3 +547,21 @@ void av_fast_mallocz(void *ptr, unsigned int *size, size_t min_size)
> >   {
> >       fast_malloc(ptr, size, min_size, 1);
> >   }
> > +
> > +int av_size_mult(size_t a, size_t b, size_t *r)
> > +{
> > +    size_t t;
> > +
> > +#if (!defined(__INTEL_COMPILER) && AV_GCC_VERSION_AT_LEAST(5,1)) || AV_HAS_BUILTIN(__builtin_mul_overflow)
> > +    if (__builtin_mul_overflow(a, b, &t))
> > +        return AVERROR(EINVAL);
> > +#else
> > +    t = a * b;
> > +    /* Hack inspired from glibc: don't try the division if nelem and elsize
> > +     * are both less than sqrt(SIZE_MAX). */
> > +    if ((a | b) >= ((size_t)1 << (sizeof(size_t) * 4)) && a && t / a != b)
> > +        return AVERROR(EINVAL);
> > +#endif
> > +    *r = t;
> > +    return 0;
> > +}
> > diff --git a/libavutil/mem.h b/libavutil/mem.h
> > index c876111afb..e9d343eaf0 100644
> > --- a/libavutil/mem.h
> > +++ b/libavutil/mem.h
> > @@ -31,7 +31,6 @@
> >   #include <stdint.h>
> >   
> >   #include "attributes.h"
> > -#include "error.h"
> >   #include "avutil.h"
> >   #include "version.h"
> >   
> > @@ -672,23 +671,7 @@ void *av_dynarray2_add(void **tab_ptr, int *nb_ptr, size_t elem_size,
> >    * @param[out] r   Pointer to the result of the operation
> >    * @return 0 on success, AVERROR(EINVAL) on overflow
> >    */
> 
> You should also move the doxy.

Why?

On 6/10/2021 12:01 PM, Anton Khirnov wrote:
> Quoting James Almer (2021-05-31 13:56:20)
>> On 5/31/2021 6:26 AM, Anton Khirnov wrote:
>>> There seems to be no compelling reason for it to be inline.
>>> ---
>>>    libavutil/mem.c | 18 ++++++++++++++++++
>>>    libavutil/mem.h | 19 +------------------
>>>    2 files changed, 19 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/libavutil/mem.c b/libavutil/mem.c
>>> index a52d33d4a6..063635fb22 100644
>>> --- a/libavutil/mem.c
>>> +++ b/libavutil/mem.c
>>> @@ -547,3 +547,21 @@ void av_fast_mallocz(void *ptr, unsigned int *size, size_t min_size)
>>>    {
>>>        fast_malloc(ptr, size, min_size, 1);
>>>    }
>>> +
>>> +int av_size_mult(size_t a, size_t b, size_t *r)
>>> +{
>>> +    size_t t;
>>> +
>>> +#if (!defined(__INTEL_COMPILER) && AV_GCC_VERSION_AT_LEAST(5,1)) || AV_HAS_BUILTIN(__builtin_mul_overflow)
>>> +    if (__builtin_mul_overflow(a, b, &t))
>>> +        return AVERROR(EINVAL);
>>> +#else
>>> +    t = a * b;
>>> +    /* Hack inspired from glibc: don't try the division if nelem and elsize
>>> +     * are both less than sqrt(SIZE_MAX). */
>>> +    if ((a | b) >= ((size_t)1 << (sizeof(size_t) * 4)) && a && t / a != b)
>>> +        return AVERROR(EINVAL);
>>> +#endif
>>> +    *r = t;
>>> +    return 0;
>>> +}
>>> diff --git a/libavutil/mem.h b/libavutil/mem.h
>>> index c876111afb..e9d343eaf0 100644
>>> --- a/libavutil/mem.h
>>> +++ b/libavutil/mem.h
>>> @@ -31,7 +31,6 @@
>>>    #include <stdint.h>
>>>    
>>>    #include "attributes.h"
>>> -#include "error.h"
>>>    #include "avutil.h"
>>>    #include "version.h"
>>>    
>>> @@ -672,23 +671,7 @@ void *av_dynarray2_add(void **tab_ptr, int *nb_ptr, size_t elem_size,
>>>     * @param[out] r   Pointer to the result of the operation
>>>     * @return 0 on success, AVERROR(EINVAL) on overflow
>>>     */
>>
>> You should also move the doxy.
> 
> Why?

Sorry, for whatever reason i read this as moving the function to mem.c 
and not just the implementation (Like i did with ff_fast_malloc).