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

[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.

> 
> >
> > -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".