* [FFmpeg-devel] [PATCH] avcodec/mjpegdec: fix -Wparentheses warning
@ 2022-09-23 17:40 Zhao Zhili
2022-09-23 10:10 ` Steven Liu
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Zhao Zhili @ 2022-09-23 17:40 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Zhao Zhili
From: Zhao Zhili <zhilizhao@tencent.com>
Signed-off-by: Zhao Zhili <zhilizhao@tencent.com>
---
libavcodec/mjpegdec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index c594950500..d77c644d3b 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -2866,7 +2866,7 @@ the_end:
}
}
- if (e = av_dict_get(s->exif_metadata, "Orientation", e, AV_DICT_IGNORE_SUFFIX)) {
+ if ((e = av_dict_get(s->exif_metadata, "Orientation", e, AV_DICT_IGNORE_SUFFIX))) {
char *value = e->value + strspn(e->value, " \n\t\r"), *endptr;
int orientation = strtol(value, &endptr, 0);
--
2.25.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [FFmpeg-devel] [PATCH] avcodec/mjpegdec: fix -Wparentheses warning
2022-09-23 17:40 [FFmpeg-devel] [PATCH] avcodec/mjpegdec: fix -Wparentheses warning Zhao Zhili
@ 2022-09-23 10:10 ` Steven Liu
2022-09-23 14:35 ` Rémi Denis-Courmont
2022-09-23 15:11 ` Andreas Rheinhardt
2 siblings, 0 replies; 7+ messages in thread
From: Steven Liu @ 2022-09-23 10:10 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Zhao Zhili
Zhao Zhili <quinkblack@foxmail.com> 于2022年9月23日周五 17:40写道：
>
> From: Zhao Zhili <zhilizhao@tencent.com>
>
> Signed-off-by: Zhao Zhili <zhilizhao@tencent.com>
> ---
> libavcodec/mjpegdec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
lgtm
>
> diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
> index c594950500..d77c644d3b 100644
> --- a/libavcodec/mjpegdec.c
> +++ b/libavcodec/mjpegdec.c
> @@ -2866,7 +2866,7 @@ the_end:
> }
> }
>
> - if (e = av_dict_get(s->exif_metadata, "Orientation", e, AV_DICT_IGNORE_SUFFIX)) {
> + if ((e = av_dict_get(s->exif_metadata, "Orientation", e, AV_DICT_IGNORE_SUFFIX))) {
> char *value = e->value + strspn(e->value, " \n\t\r"), *endptr;
> int orientation = strtol(value, &endptr, 0);
>
> --
> 2.25.1
>
> _______________________________________________
> 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".
_______________________________________________
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".
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [FFmpeg-devel] [PATCH] avcodec/mjpegdec: fix -Wparentheses warning
2022-09-23 17:40 [FFmpeg-devel] [PATCH] avcodec/mjpegdec: fix -Wparentheses warning Zhao Zhili
2022-09-23 10:10 ` Steven Liu
@ 2022-09-23 14:35 ` Rémi Denis-Courmont
2022-09-23 17:46 ` Zhao Zhili
2022-09-23 15:11 ` Andreas Rheinhardt
2 siblings, 1 reply; 7+ messages in thread
From: Rémi Denis-Courmont @ 2022-09-23 14:35 UTC (permalink / raw)
To: ffmpeg-devel
Le perjantaina 23. syyskuuta 2022, 20.40.30 EEST Zhao Zhili a écrit :
> From: Zhao Zhili <zhilizhao@tencent.com>
>
> Signed-off-by: Zhao Zhili <zhilizhao@tencent.com>
> ---
> libavcodec/mjpegdec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
> index c594950500..d77c644d3b 100644
> --- a/libavcodec/mjpegdec.c
> +++ b/libavcodec/mjpegdec.c
> @@ -2866,7 +2866,7 @@ the_end:
> }
> }
>
> - if (e = av_dict_get(s->exif_metadata, "Orientation", e,
> AV_DICT_IGNORE_SUFFIX)) { + if ((e = av_dict_get(s->exif_metadata,
> "Orientation", e, AV_DICT_IGNORE_SUFFIX))) { char *value = e->value +
> strspn(e->value, " \n\t\r"), *endptr; int orientation = strtol(value,
> &endptr, 0);
Isn't it easier to read if you break the assignment out, and use the result as
predicate??
--
レミ・デニ-クールモン
http://www.remlab.net/
_______________________________________________
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".
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [FFmpeg-devel] [PATCH] avcodec/mjpegdec: fix -Wparentheses warning
2022-09-23 14:35 ` Rémi Denis-Courmont
@ 2022-09-23 17:46 ` Zhao Zhili
2022-09-28 14:45 ` Anton Khirnov
0 siblings, 1 reply; 7+ messages in thread
From: Zhao Zhili @ 2022-09-23 17:46 UTC (permalink / raw)
To: 'FFmpeg development discussions and patches'
> -----Original Message-----
> From: ffmpeg-devel-bounces@ffmpeg.org <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Rémi Denis-Courmont
> Sent: 2022年9月23日 22:35
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] avcodec/mjpegdec: fix -Wparentheses warning
>
> Le perjantaina 23. syyskuuta 2022, 20.40.30 EEST Zhao Zhili a écrit :
> > From: Zhao Zhili <zhilizhao@tencent.com>
> >
> > Signed-off-by: Zhao Zhili <zhilizhao@tencent.com>
> > ---
> > libavcodec/mjpegdec.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
> > index c594950500..d77c644d3b 100644
> > --- a/libavcodec/mjpegdec.c
> > +++ b/libavcodec/mjpegdec.c
> > @@ -2866,7 +2866,7 @@ the_end:
> > }
> > }
> >
> > - if (e = av_dict_get(s->exif_metadata, "Orientation", e,
> > AV_DICT_IGNORE_SUFFIX)) { + if ((e = av_dict_get(s->exif_metadata,
> > "Orientation", e, AV_DICT_IGNORE_SUFFIX))) { char *value = e->value +
> > strspn(e->value, " \n\t\r"), *endptr; int orientation = strtol(value,
> > &endptr, 0);
>
> Isn't it easier to read if you break the assignment out, and use the result as
> predicate??
I prefer the coding style of splitting into two lines personally, but current coding
Style is used everywhere in the code base, so I'm not sure.
Since -Wparentheses has been disabled explicitly, the patch doesn't matter now.
>
> --
> レミ・デニ-クールモン
> http://www.remlab.net/
>
>
>
> _______________________________________________
> 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".
_______________________________________________
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".
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [FFmpeg-devel] [PATCH] avcodec/mjpegdec: fix -Wparentheses warning
2022-09-23 17:46 ` Zhao Zhili
@ 2022-09-28 14:45 ` Anton Khirnov
0 siblings, 0 replies; 7+ messages in thread
From: Anton Khirnov @ 2022-09-28 14:45 UTC (permalink / raw)
To: 'FFmpeg development discussions and patches'
Quoting Zhao Zhili (2022-09-23 19:46:24)
>
>
> > -----Original Message-----
> > From: ffmpeg-devel-bounces@ffmpeg.org <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Rémi Denis-Courmont
> > Sent: 2022年9月23日 22:35
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH] avcodec/mjpegdec: fix -Wparentheses warning
> >
> > Le perjantaina 23. syyskuuta 2022, 20.40.30 EEST Zhao Zhili a écrit :
> > > From: Zhao Zhili <zhilizhao@tencent.com>
> > >
> > > Signed-off-by: Zhao Zhili <zhilizhao@tencent.com>
> > > ---
> > > libavcodec/mjpegdec.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
> > > index c594950500..d77c644d3b 100644
> > > --- a/libavcodec/mjpegdec.c
> > > +++ b/libavcodec/mjpegdec.c
> > > @@ -2866,7 +2866,7 @@ the_end:
> > > }
> > > }
> > >
> > > - if (e = av_dict_get(s->exif_metadata, "Orientation", e,
> > > AV_DICT_IGNORE_SUFFIX)) { + if ((e = av_dict_get(s->exif_metadata,
> > > "Orientation", e, AV_DICT_IGNORE_SUFFIX))) { char *value = e->value +
> > > strspn(e->value, " \n\t\r"), *endptr; int orientation = strtol(value,
> > > &endptr, 0);
> >
> > Isn't it easier to read if you break the assignment out, and use the result as
> > predicate??
>
> I prefer the coding style of splitting into two lines personally, but current coding
> Style is used everywhere in the code base, so I'm not sure.
Not everywhere, it depends on the author.
I try to avoid assignments in conditions wherever possible, because they
are hard to read IMO.
--
Anton Khirnov
_______________________________________________
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".
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/mjpegdec: fix -Wparentheses warning
2022-09-23 17:40 [FFmpeg-devel] [PATCH] avcodec/mjpegdec: fix -Wparentheses warning Zhao Zhili
2022-09-23 10:10 ` Steven Liu
2022-09-23 14:35 ` Rémi Denis-Courmont
@ 2022-09-23 15:11 ` Andreas Rheinhardt
2022-09-23 17:43 ` Zhao Zhili
2 siblings, 1 reply; 7+ messages in thread
From: Andreas Rheinhardt @ 2022-09-23 15:11 UTC (permalink / raw)
To: ffmpeg-devel
Zhao Zhili:
> From: Zhao Zhili <zhilizhao@tencent.com>
>
> Signed-off-by: Zhao Zhili <zhilizhao@tencent.com>
> ---
> libavcodec/mjpegdec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
> index c594950500..d77c644d3b 100644
> --- a/libavcodec/mjpegdec.c
> +++ b/libavcodec/mjpegdec.c
> @@ -2866,7 +2866,7 @@ the_end:
> }
> }
>
> - if (e = av_dict_get(s->exif_metadata, "Orientation", e, AV_DICT_IGNORE_SUFFIX)) {
> + if ((e = av_dict_get(s->exif_metadata, "Orientation", e, AV_DICT_IGNORE_SUFFIX))) {
> char *value = e->value + strspn(e->value, " \n\t\r"), *endptr;
> int orientation = strtol(value, &endptr, 0);
>
Since when do we care about this type of warning? Line 7087 of configure
adds -Wno-parentheses in case the compiler supports it.
- Andreas
_______________________________________________
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".
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [FFmpeg-devel] [PATCH] avcodec/mjpegdec: fix -Wparentheses warning
2022-09-23 15:11 ` Andreas Rheinhardt
@ 2022-09-23 17:43 ` Zhao Zhili
0 siblings, 0 replies; 7+ messages in thread
From: Zhao Zhili @ 2022-09-23 17:43 UTC (permalink / raw)
To: 'FFmpeg development discussions and patches'
> -----Original Message-----
> From: ffmpeg-devel-bounces@ffmpeg.org <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Andreas Rheinhardt
> Sent: 2022年9月23日 23:11
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] avcodec/mjpegdec: fix -Wparentheses warning
>
> Zhao Zhili:
> > From: Zhao Zhili <zhilizhao@tencent.com>
> >
> > Signed-off-by: Zhao Zhili <zhilizhao@tencent.com>
> > ---
> > libavcodec/mjpegdec.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
> > index c594950500..d77c644d3b 100644
> > --- a/libavcodec/mjpegdec.c
> > +++ b/libavcodec/mjpegdec.c
> > @@ -2866,7 +2866,7 @@ the_end:
> > }
> > }
> >
> > - if (e = av_dict_get(s->exif_metadata, "Orientation", e, AV_DICT_IGNORE_SUFFIX)) {
> > + if ((e = av_dict_get(s->exif_metadata, "Orientation", e, AV_DICT_IGNORE_SUFFIX))) {
> > char *value = e->value + strspn(e->value, " \n\t\r"), *endptr;
> > int orientation = strtol(value, &endptr, 0);
> >
>
> Since when do we care about this type of warning? Line 7087 of configure
> adds -Wno-parentheses in case the compiler supports it.
OK. Before sending the patch, I have checked git log that there is a commit 3fb32ae2
which fixed a bug catched by -Wparentheses. I didn't notice -Wparentheses has been
disabled explicitly since too much false positive. On the other hand, I'd like to suggest
to avoid such coding style for new code.
Please ignore the patch itself.
>
> - Andreas
> _______________________________________________
> 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".
_______________________________________________
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".
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-09-28 14:45 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-23 17:40 [FFmpeg-devel] [PATCH] avcodec/mjpegdec: fix -Wparentheses warning Zhao Zhili
2022-09-23 10:10 ` Steven Liu
2022-09-23 14:35 ` Rémi Denis-Courmont
2022-09-23 17:46 ` Zhao Zhili
2022-09-28 14:45 ` Anton Khirnov
2022-09-23 15:11 ` Andreas Rheinhardt
2022-09-23 17:43 ` Zhao Zhili
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git