From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 4905F42F8A for ; Mon, 16 May 2022 22:03:13 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 1D2AD68B425; Tue, 17 May 2022 01:03:11 +0300 (EEST) Received: from NAM04-BN8-obe.outbound.protection.outlook.com (mail-bn8nam08olkn2098.outbound.protection.outlook.com [40.92.47.98]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 3D82F6800E0 for ; Tue, 17 May 2022 01:03:05 +0300 (EEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=TwA9llr0LGCmFXOdmPdD5zPO4wogfOaKQbv8iF5kXO1CsNrsuzlO533YB5kNv1TBtcqbQgZ1MyxmxHMLev+AgttMAADUhAwnDOCPmwj77s7+2txlLQMkGaxbHHa8oR6L1Ep9zAGvoMAV8e0R3sjNA4pHhuZsFI9mHOmjXpF0c0m9susvAoOLYdeTUNvS+xrKHPQjLHTC9MnWWK2OUi2OZOo2qKg4j1Gi56E2o+VX/Jid9+9J8qWWyzl+ApImsAqMph0sZBcfmPgm5GFnFvDoOBk99G6JJgb++qq1maLbpBCK6UBrPbiG2jrwdd5+yZ5kf10887QW/kXN+7JRhWxwzw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=rzpLtTGVsMGD0UhX5tSdkd8vGNfBvMDXePXeYEZDEPE=; b=nFEH+fHeq3lW7OX0Tes5GqwKDq9ZnxutxpNmOKuPWJ9mrKUYSJcZ6I3VOb6fHMz7k0aqsoTS5ZWykaL/PbOtgzph1ZTE3S/OCjAvAODf+froCwIch9AjFoWeW5s9R3DL2fAFDnD6GX1GhvRVs8vP2gIHVVeP2rMSP/13da5KoPar9aYNmHOolN7zTtCZlnheb214jDDmlD28PVhLSVA4voBOp/drm0k8nYYlBrpHCqMltt5zDRCBA2XbhlB/G/5SkXarm8o53PonA9ywfp448R1jGxSnnUcoKx3KUgJQwHplQKqtg+oevdyOdqGjqdCdJ85XcD8H9FLVJEffbeKRyg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=hotmail.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=rzpLtTGVsMGD0UhX5tSdkd8vGNfBvMDXePXeYEZDEPE=; b=Otkb2wy2PRalHVDldngUpb89oW049QQlw/nMvNGFitqlek5nLIBKThmRnil9CZqIKvFReqTA1zU8ZD7HR6VAQamLlzUoLWzy0cw4SJlCjBFOFu+qeVDtWLLWk7R5OehtrjovbnleQBsXgJyTSwaLldkEs03ItMAxGi4xxmei4Z3X9jK4YeBALgVEgIdtPFHa3MaBwIZoO69YDcMPl1NBLBV1RSnjYqXN907Pf1RmvsC/O4rMg2sd6eOelL59SSX8M01927+6XYegK7r3RhS4nzr2yMAdHkRzTS2vag4SIC0vEtMhCxTW2D+DiBL0cXe0LCYY6JxZF4F60Akpdy6zew== Received: from DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM (2603:10b6:8:b::20) by CH0P223MB0153.NAMP223.PROD.OUTLOOK.COM (2603:10b6:610:f5::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5250.14; Mon, 16 May 2022 22:03:02 +0000 Received: from DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM ([fe80::c536:493f:7cda:53dc]) by DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM ([fe80::c536:493f:7cda:53dc%3]) with mapi id 15.20.5250.018; Mon, 16 May 2022 22:03:02 +0000 From: Soft Works To: FFmpeg development discussions and patches Thread-Topic: [FFmpeg-devel] [PATCH v4 06/10] libavformat/asfdec: fix macro definition and use Thread-Index: AQHYZ9TzhKShTiFw/kGLP3ifMRIxzK0gPvYAgABHRaCAAK2qgIAA15TA Date: Mon, 16 May 2022 22:03:02 +0000 Message-ID: References: <33b3d163dfcd61ae6f4ac258ae28fa0756436587.1652561722.git.ffmpegagent@gmail.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-tmn: [iNUvwHTCh6RJgz+Kh28Rxog5aQ7FYoWq] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 83f81490-197c-4062-bf9d-08da3787d8c2 x-ms-traffictypediagnostic: CH0P223MB0153:EE_ x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: PZ0kn3PlKArQs0wQDwxi67tl9FBqG4eLlbvlpGboZgSlEcj+heXKVXa+TfxSdMkKFPmqabIPw0JBSOWOVNxwGRQnmDsOy5PiJrFbK9N5zRALFp0AZpWhxJjK4IJs8wEC1eoLD7NVNr8BhleUwLuCD87J+fXj0cB1U3DlntgI+1JY2y4WNhpNSyHg0vdLnDJN+eEizLpJffiRhnomM21B1V3orKEO0pYRNEt8BhK9InbzJMfsSUd0LdNCXDaTB3o5SWGRzQWhgjSPyQgaE/slGRx+j3SQr2JT32A4xt+ngERsfg9GxLE/tBw5N/QtMiGw6qGlcNxZT1WOcCeqpu3edbdpXSeXzBkLC8o1rv8tVHNv/IdEYUpCHlhhflbVsxmc8xCzneJPrQnAw6A7p7XuaRwUTekSF8TY1cgS7SBVwcUcEuCyO9yqcuADaH9AYku6qvdfRmuru6jyLik2mmYof00kN1/yG4h9GDpTmAbxm1GKoc9NSoRNLt277SORIhNynJKPHv8mjOt+ITC2Pb3eKjn/kZctmr1IfbqU0Wek5GLLS8tBk9TqsI+auM4+jFT5uh5bkkTl/d5yeYpkfSY+C7I9B3tez6ebSgwC2ijBRCTD+/4Dx6uXYji7sw5OhNRsoRs9NKwh68YzaX5CBc0+y9UK4YkKL0BYISoPeR/gR9Q= x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?utf-8?B?MTRxQ1lDN0xmQ0NWeXVaakkzc3VmeHJzaXRpTldVRlNhM0lqMXNZb2JEWEdj?= =?utf-8?B?dG1ITVZXTEdyU0NPUlBZcXJwdldKQXVybmdTQUh1OFdsV3RyMC9kRU1pYU51?= =?utf-8?B?QS90K3dqKytHVlVzZHZTa001eXpCRGhBQlh0N2V6SDhhTW04WURCdndrY0JC?= =?utf-8?B?d3A3azI5YlJGWjU4QW5zTEVOVnRsUWxhV2VoT1FIY0ZFU1Z4QnloSEZmdUxE?= =?utf-8?B?SlpVQ2Y5MXAwYzl4NmhDTTB5SUN1YnJHN0ZMVWpnTXJMZUdwRjZkUkpqZlJs?= =?utf-8?B?WW85bTBZNWdZMDlxNzFJWmcxVmFlcTI5eHd0WkFZM0JQN0NheTBmbEswYTE4?= =?utf-8?B?OXN5L25jd1NhMTZkSDJaZmExb1ErRUFaeDA5YU9iQ25xOUJRTmFIaEZpdlpK?= =?utf-8?B?c1pjZHAwa0ZTWTRZNlBzZ0lueHZqK2hDY2NvMGpGc0NCMDJJdGFNcW5XbStq?= =?utf-8?B?V2tCbFhxdmxiRUJyUG53aitQcDRMcWpiMEJ3MlJXbnN3ckxpb2RZbFZob3FS?= =?utf-8?B?c3NRT2J0S0h4SUYvT2kyWXhrK3dmMWZMSFpTOE5uVlhITlFscCtBTE5kVjN3?= =?utf-8?B?WXQ4NVdGb3F2ZHJnTTV2NmVYRWlZWUFsT1BtOXlpYnlJNnd4Tlk1bjkvYWZB?= =?utf-8?B?dTJ5V2JxaFQ4bCt4MzhQbHJBbjNaSmdMYms5STFCd0dqOHRLTXNSeW9nMEY0?= =?utf-8?B?ZUoxRTlURlg0eDQ3dUoySml2RW5NSDVKeFo5clA2RVIzbFd2Y2h4NWRWajNi?= =?utf-8?B?QS8za0grbmp0NzFHSitCbUYrNndldmhrdnhvMGdGRXNhS0pTUHh6N0hBdEY0?= =?utf-8?B?OTllemdzMExtajZIT2JkWDNNbUJQQ0RUU2lkOVJOVm96a1VJcVF5eE8yRHBV?= =?utf-8?B?ak1Nd3hPdThQT09udEFNelRSN1hwZ3pWUkV2M1ZzM3BXb1ZZNEdLclZEaU1P?= =?utf-8?B?am9MTmZsTXZsR01BeG11aE1RYzQvOW9rKy9FWmpyMDVqUFlJQ24zb0ExQlZk?= =?utf-8?B?U1ZUQUgrS2VKa25XWWhMZ05VbXRoSUtrSFNXZEllSEw4dC9HR2wzdEdORFJL?= =?utf-8?B?K0lDV2lFSGpDNlE4VTVldU1nbVphY3ZVZ3RvbWNySkZpOTM1OEFQMDdYb1JR?= =?utf-8?B?UFBFck44Q0l0TWhpektQU2hIZGEyUysyN3JFdWQ0MStwTE5TTzhlMFRzZ1Zp?= =?utf-8?B?VHlvTi9TWmwxajZQWVJhb2pDbXVuWFplQ0ZZNzY5ZUs4N1FzcVZVN1FCTzZN?= =?utf-8?B?eTdsVHBseGFld3ZvaHcvS3YrTkt0ZXprdjVsM3RIRlc5Nlk0SnY2dVpnRkVl?= =?utf-8?B?VlcrTjNqb09lN3BUTW8vdHpQUThaSVVuY3VIWTR4eis4UE1IU0FIenFIbUtv?= =?utf-8?B?aEx3WVBhdnhzR2RPL2VHaDZBc3FxZE5OSkplYnBsbU1US3A1MTRoSDB5QWxM?= =?utf-8?B?a3M5ZVRwQ041WHRrSWR1R1R5YkZ6TzFpTzk1Y3NjaFk0Z3dxUEhFQjZGR1Vh?= =?utf-8?B?cnFKU0dXSDExR3FCM0szQ0xtdzBTVGRtVzNSMWZZVGcrTDhtdnh5dUlYU1Nw?= =?utf-8?B?OENhOVU5anVxNWRxTE90Kys1ZHRzdmk1QmxIWU1OSForMHVFcUplK2tvcm1N?= =?utf-8?B?ZGI1cmt3dkk0Y1JUcDhsLy94ODFPOU5qdjRFU1ZzME9kOGwwVVM2aDBubUFm?= =?utf-8?B?M25hNWN6cnAxVDduOUU1cVNhMzk5N3E2MXoveCtCRnlSY2dwOXMwWldsVzFP?= =?utf-8?B?U2VZejVlblFsRU5GZWFqNGUrMnMzV09wTHlabm5NQUhJTVN1MGtINWRoVEZy?= =?utf-8?B?NUtqVHMrL1VOUkhzd1p6am5XU2RZd05mdVQ1ZlZKTEl2Qk93R2lkYkZ5Qkkz?= =?utf-8?B?bC9GK3V6YVVzK3VqTEtoVmlXN0UvKzhNblkzVDFvaXl6Z0VlSlRJTnlrSG1i?= =?utf-8?Q?hhzRY5AKxDE=3D?= MIME-Version: 1.0 X-OriginatorOrg: sct-15-20-4755-11-msonline-outlook-1ff67.templateTenant X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-CrossTenant-Network-Message-Id: 83f81490-197c-4062-bf9d-08da3787d8c2 X-MS-Exchange-CrossTenant-originalarrivaltime: 16 May 2022 22:03:02.2632 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-rms-persistedconsumerorg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0P223MB0153 Subject: Re: [FFmpeg-devel] [PATCH v4 06/10] libavformat/asfdec: fix macro definition and use X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: > -----Original Message----- > From: ffmpeg-devel On Behalf Of > Andreas Rheinhardt > Sent: Monday, May 16, 2022 10:49 AM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH v4 06/10] libavformat/asfdec: fix > macro definition and use > > Soft Works: > > > > > >> -----Original Message----- > >> From: ffmpeg-devel On Behalf Of > >> Andreas Rheinhardt > >> Sent: Sunday, May 15, 2022 8:12 PM > >> To: ffmpeg-devel@ffmpeg.org > >> Subject: Re: [FFmpeg-devel] [PATCH v4 06/10] libavformat/asfdec: > fix > >> macro definition and use > >> > >> softworkz: > >>> From: softworkz > >>> > >>> Signed-off-by: softworkz > >>> --- > >>> libavformat/asfdec_f.c | 24 ++++++++++++------------ > >>> 1 file changed, 12 insertions(+), 12 deletions(-) > >>> > >>> diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c > >>> index 81a29f99d5..91c3874ac7 100644 > >>> --- a/libavformat/asfdec_f.c > >>> +++ b/libavformat/asfdec_f.c > >>> @@ -906,21 +906,21 @@ static int asf_read_header(AVFormatContext > *s) > >>> } > >>> > >>> #define DO_2BITS(bits, var, defval) \ > >>> - switch (bits & 3) { \ > >>> + switch ((bits) & 3) { \ > >>> case 3: \ > >>> - var = avio_rl32(pb); \ > >>> + (var) = avio_rl32(pb); \ > >>> rsize += 4; \ > >>> break; \ > >>> case 2: \ > >>> - var = avio_rl16(pb); \ > >>> + (var) = avio_rl16(pb); \ > >>> rsize += 2; \ > >>> break; \ > >>> case 1: \ > >>> - var = avio_r8(pb); \ > >>> + (var) = avio_r8(pb); \ > >>> rsize++; \ > >>> break; \ > >>> default: \ > >>> - var = defval; \ > >>> + (var) = (defval); \ > >>> break; \ > >>> } > >>> > >>> @@ -1003,9 +1003,9 @@ static int asf_get_packet(AVFormatContext > *s, > >> AVIOContext *pb) > >>> asf->packet_flags = c; > >>> asf->packet_property = d; > >>> > >>> - DO_2BITS(asf->packet_flags >> 5, packet_length, s- > >>> packet_size); > >>> - DO_2BITS(asf->packet_flags >> 1, padsize, 0); // sequence > >> ignored > >>> - DO_2BITS(asf->packet_flags >> 3, padsize, 0); // padding > length > >>> + DO_2BITS(asf->packet_flags >> 5, packet_length, s- > >packet_size) > >>> + DO_2BITS(asf->packet_flags >> 1, padsize, 0) // sequence > >> ignored > >>> + DO_2BITS(asf->packet_flags >> 3, padsize, 0) // padding > length > >>> > >>> // the following checks prevent overflows and infinite loops > >>> if (!packet_length || packet_length >= (1U << 29)) { > >>> @@ -1066,9 +1066,9 @@ static int > >> asf_read_frame_header(AVFormatContext *s, AVIOContext *pb) > >>> asf->stream_index = asf->asfid2avid[num & 0x7f]; > >>> asfst = &asf->streams[num & 0x7f]; > >>> // sequence should be ignored! > >>> - DO_2BITS(asf->packet_property >> 4, asf->packet_seq, 0); > >>> - DO_2BITS(asf->packet_property >> 2, asf->packet_frag_offset, > >> 0); > >>> - DO_2BITS(asf->packet_property, asf->packet_replic_size, 0); > >>> + DO_2BITS(asf->packet_property >> 4, asf->packet_seq, 0) > >>> + DO_2BITS(asf->packet_property >> 2, asf->packet_frag_offset, > 0) > >>> + DO_2BITS(asf->packet_property, asf->packet_replic_size, 0) > >>> av_log(asf, AV_LOG_TRACE, "key:%d stream:%d seq:%d offset:%d > >> replic_size:%d num:%X packet_property %X\n", > >>> asf->packet_key_frame, asf->stream_index, asf- > >>> packet_seq, > >>> asf->packet_frag_offset, asf->packet_replic_size, > num, > >> asf->packet_property); > >>> @@ -1144,7 +1144,7 @@ static int > >> asf_read_frame_header(AVFormatContext *s, AVIOContext *pb) > >>> return AVERROR_INVALIDDATA; > >>> } > >>> if (asf->packet_flags & 0x01) { > >>> - DO_2BITS(asf->packet_segsizetype >> 6, asf- > >>> packet_frag_size, 0); // 0 is illegal > >>> + DO_2BITS(asf->packet_segsizetype >> 6, asf- > >>> packet_frag_size, 0) // 0 is illegal > >>> if (rsize > asf->packet_size_left) { > >>> av_log(s, AV_LOG_ERROR, "packet_replic_size is > >> invalid\n"); > >>> return AVERROR_INVALIDDATA; > >> > >> While protecting macro arguments is good, it is not really a "fix" > >> unless current usage is buggy. > > > > Ok, I will rephrase the commit message. > > > >> Which it isn't here, because >> has higher precedence than &. > > > > Could you explain which change you are referring to? > > > > Putting "bits" in parentheses. It doesn't change anything, because >> > has higher precedence than &. Ah, that's what you mean. I didn't even look at the usages of the macro, because I think a macro should be safe intrinsically, not only based on its current usages. Actually this had also caught my attention due to a clang warning: https://releases.llvm.org/13.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/bugprone-macro-parentheses.html > > All this patch does is to put macro variables in brackets > > and remove semicolons.. > > > >> Furthermore I am not really sure whether removing the ';' is even > >> something worthwhile; they are surely unnecessary (being null > >> statements), but does this matter? > > > > It causes a warning > > > > > https://releases.llvm.org/13.0.0/tools/clang/docs/DiagnosticsReference > .html#wextra-semi-stmt > > > > I don't receive this warning despite using Clang 13.0. Do you have - > Wall > or -Wextra or something like that enabled? I'm using ReSharper C++ which is using clang-tidy from clang 13.0 with -Weverything What settings do you use? > IMO a better fix for this would be to wrap the macro in a do {} while > (0) to keep the macro calls function-like. Isn't that a bit too... hm.. much/ugly? > Anyway, you should have mentioned in the commit message that your aim > is to fix this uncommon warning. Yes, that makes sense. > > I don't know how others are working, but I use to work in a way > where > > such warnings are shown in the editor and in lists in the IDE > > even without compilation. Now - when you have a code file that > > generates like 20, 50 or more warnings, it's much harder to spot > > those warnings that might be really relevant and hinting at a > mistake, > > and you might be just too lazy to go through them each time. > > > > The clang diagnostics have been helpful in spotting some actual > > issues in this very file. That's why I consider it worthwhile > > to also eliminate such "non-issues". > > > > I also work like that; e.g. my recent ac3.h header patchset was > inspired > by clangd not liking cycles in header inclusions ("In included file: > main file cannot be included recursively when building a preamble"). Yea, I gathered from some of your patches that you must be using some tooling as well. Would you allow me the question which IDE you are using? Thanks a lot, softworkz _______________________________________________ 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".