[FFmpeg-devel] [PATCH 1/3] dnn/queue: fix redefining typedefs

[Andreas Rheinhardt]

Guo, Yejun:
> 
> 
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
>> Andreas Rheinhardt
>> Sent: 2020年12月30日 18:04
>> To: ffmpeg-devel at ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH 1/3] dnn/queue: fix redefining typedefs
>>
>> Guo, Yejun:
>>> Signed-off-by: Guo, Yejun <yejun.guo at intel.com>
>>> ---
>>>  libavfilter/dnn/queue.c      | 8 ++++----
>>>  libavfilter/dnn/safe_queue.c | 4 ++--
>>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/libavfilter/dnn/queue.c b/libavfilter/dnn/queue.c index
>>> 0a07c5473d..da0517968d 100644
>>> --- a/libavfilter/dnn/queue.c
>>> +++ b/libavfilter/dnn/queue.c
>>> @@ -25,17 +25,17 @@
>>>
>>>  typedef struct _queue_entry queue_entry;
>>
>> There is no point in using different type names and struct tags; and both the
>> type name as well as the struct tag do not abide by our naming
>> conventions: We use CamelCase for them. Remember
>> 9beaf536fe5b52ed5af4d4dd5746277ee5ac9552? The deeper reason for this
>> was that the typedef name "thread_param" was masked by a variable of the
>> same name (of type thread_param *) and so sizeof(thread_param) referred to
>> the variable size (i.e. pointer size). Using sizeof with parentheses didn't change
>> this. If you had used our naming convention, this would have never happened.
> thanks, got it. I'll also change thread_param in another patch after this regression is fixed.
> 
>>
>> And both the non-internal type as well as the function names need the correct
>> prefix.
> thanks, I'll add dnn_ as prefix for queue, safe_queue, and the relative functions.
> 

The correct prefix for symbols not exported from the library and not
local to one translation unit is ff_ (or FF for types).

>>
>>>
>>> -typedef struct _queue {
>>> +struct _queue {
>>>      queue_entry *head;
>>>      queue_entry *tail;
>>>      size_t length;
>>> -}queue;
>>> +};
>>
>> It is more logical to have this definition after the entry's definition.
> will change it, thanks.
> 
>>
>>>
>>> -typedef struct _queue_entry {
>>> +struct _queue_entry {
>>>      void *value;
>>>      queue_entry *prev;
>>>      queue_entry *next;
>>> -} queue_entry;
>>> +};
>>>
>>>  static inline queue_entry *create_entry(void *val)  { diff --git
>>> a/libavfilter/dnn/safe_queue.c b/libavfilter/dnn/safe_queue.c index
>>> dba2e0fbbc..4298048454 100644
>>> --- a/libavfilter/dnn/safe_queue.c
>>> +++ b/libavfilter/dnn/safe_queue.c
>>> @@ -25,11 +25,11 @@
>>>  #include "libavutil/avassert.h"
>>>  #include "libavutil/thread.h"
>>>
>>> -typedef struct _safe_queue {
>>> +struct _safe_queue {
>>>      queue *q;
>>>      pthread_mutex_t mutex;
>>>      pthread_cond_t cond;
>>> -}safe_queue;
>>> +};
>>>
>>>  safe_queue *safe_queue_create(void)
>>>  {
>>>
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email ffmpeg-devel-request at ffmpeg.org
>> with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>