[FFmpeg-devel] [PATCH 1/3] dnn/queue: fix redefining typedefs
Guo, Yejun
yejun.guo at intel.com
Wed Dec 30 15:04:15 EET 2020
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Guo,
> Yejun
> Sent: 2020年12月30日 20:58
> To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/3] dnn/queue: fix redefining typedefs
>
>
>
> > -----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.
DNNQueue, DNNSafeQueue, dnn_queue_create, etc
>
> >
> > >
> > > -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".
More information about the ffmpeg-devel
mailing list