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 408B742F3E for ; Mon, 16 May 2022 08:49:09 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id DEF2268B50A; Mon, 16 May 2022 11:49:07 +0300 (EEST) Received: from EUR05-DB8-obe.outbound.protection.outlook.com (mail-db8eur05olkn2077.outbound.protection.outlook.com [40.92.89.77]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id E671168B2DC for ; Mon, 16 May 2022 11:49:01 +0300 (EEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=CcMAJ9MO8lCkS54LDzgvUgdOUq7g1VYpK1qAC0tRL3SBLAKOJ2GcF17KNT64N/OtJ8kBA5xDxXWz8rt7ogTnb2fZcxIzi4vapMhdMdSRvOURRaAMs2kvTkZWWJZ7/NyjAzLX/LkDwzdF0zR1evvNpo9FZCG2F0Ngu54wqBS6W1bus/wqEZsey8BjIGPcF3ju4nwScEL+OE3r4DzqT/2Ut2VhlqDpaRcocWWJbye7Jx3+imYFIaOhRZitUUCRr/NM7BaLiXYYMX19fw1TIIGoZBYo0kQQrYxwoLoVrc37NosHBuXc9ZIU88SSESG2oc2Yr9JkAoMMvhte9NFzgmW3xA== 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=slloW23G7FRDef9DlDiEF/TbDDxfsUzduiGU4nzj0Yc=; b=J7T41wPPiqnWjVvBR9Jq8K24d3wcGpr9S9ZAy9E34JdtVipTcmNukueGz0PyB1zoFFY307CaQKpPlsRouLSsarzOe5m0lCCxqMEtjAgdF70iTBUAZs65UER63PItyLSLyxgEDff9SPZ1CDQY5+U1iVS/C97GTJ9sdFlZYeguo8RaF/5Hb9RyNRSj8qP/tQowlq6lS/ct2iZVdyxEovs0otPHhSVuihHkadRF5tuEj7fo1JPgnA+leL0iVgI6IKTAqshshwdfYIsSGxlV0HhqWffEApT3iD1tM1sUy25JJbX1P2sKx0ZOOoBQD6TwewLj/qjapx2BK7AfiiYbwIbmcQ== 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=outlook.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=slloW23G7FRDef9DlDiEF/TbDDxfsUzduiGU4nzj0Yc=; b=Yb5hg/MeXyGTAhw2vS4ggsWNbYozBJF5t8uF0zqhvRvElw6GM3IY4FoFSEhUeYdnE0CIq0ig/BC1XDPIxBkCc0Ypg2K5v+fJybFqb3LbS+Y7Rh1nVOvMs/8pHm8LACVJ4vRtuIHXnCSQDeVl9AOtiso7i0cwDlVcb6BdAZuKICr5iw/QZEfk32Utq0TM5+cKiIYFq60NBy3ax3VDr2UdSLHC3dAYyRxNSnqje+LToJVzGFGa8MRcsTwSCO2SypRnwxkU0yNhwi8CJXlaDsccuj6n4uAfmXI7VNCEkaCfKY8F5tYZvHFK0kTwWQ1gMrAD/Zh2CYkFjFnMd5wdiGfp4Q== Received: from DB6PR0101MB2214.eurprd01.prod.exchangelabs.com (2603:10a6:4:42::27) by AM9PR01MB7220.eurprd01.prod.exchangelabs.com (2603:10a6:20b:2d2::5) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5250.13; Mon, 16 May 2022 08:49:00 +0000 Received: from DB6PR0101MB2214.eurprd01.prod.exchangelabs.com ([fe80::60b9:9f29:40cc:f01c]) by DB6PR0101MB2214.eurprd01.prod.exchangelabs.com ([fe80::60b9:9f29:40cc:f01c%10]) with mapi id 15.20.5250.018; Mon, 16 May 2022 08:49:00 +0000 Message-ID: Date: Mon, 16 May 2022 10:48:57 +0200 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <33b3d163dfcd61ae6f4ac258ae28fa0756436587.1652561722.git.ffmpegagent@gmail.com> From: Andreas Rheinhardt In-Reply-To: X-TMN: [mh2Ag0fdECvdo1GWd4C9Zwiv8eTLP2Pn] X-ClientProxiedBy: ZR0P278CA0056.CHEP278.PROD.OUTLOOK.COM (2603:10a6:910:21::7) To DB6PR0101MB2214.eurprd01.prod.exchangelabs.com (2603:10a6:4:42::27) X-Microsoft-Original-Message-ID: MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: c997eca3-3522-4e5b-7485-08da3718eb9f X-MS-TrafficTypeDiagnostic: AM9PR01MB7220:EE_ X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 1Oh+qHpDE3g35V4ofv7waWy/hsJ7Yk6gt8xqj1Vz2f90IdwqnxK+d1QUhv/i8LfNW95RwzlJC+Qsglo8ei4jfVOfq0Jb85CkDCf4TfV6MNVe7Qz9YXypLANr56eLr7Bpn8Gd3SpSmS/qEa5uhASehcu5aBb5J+Z+VlP7GaCULtNlvleEIENFoKANojNrqO3SPQPnmMmD4OB4aBimYy66h54+nkIZuCe03cg+jNGDcc2UkVv6VvVNzN1vnN06uoTbRQSEjejY8TOq0qbcwb3L9HFrZEkZkAX7SZyqf80W0y43OL9FypL/ItcN5xXjixXq2JmqaCAvPNF8QmLAbUxDeb8aZupsGvdB+OsKaDzn1w21+FcV4Z4fbfK1J10AOPHEaHrjYao8z91OqXSaCazJE9xJHtOnsFjj7ECvGxpPVbtSx6QoLrku858vhz65RVOP83aKH9lK8SDzzgPDOcAb5eYEi6q0G2jbRZy/9rst6SB0wIBUhOyspVXeVKKIfmzqGOccIZCPLb81LYTETsCleGtzPrEXwMOlbuNkbaslcnVlWdjMmbVg2/R7RgOUEF/+C9sf8bDsVVLO4SdmOnukv20qJLg3WFmd3wwTbD73oA4kSLsIgO/tgaLMdi+wbhLr X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dEdlWXoySHNsMjNjM2thcFdXc0xQa0ZQOHY1Z2M5Tk1wTmJRQVZjZE1ybStQ?= =?utf-8?B?WWtmMnZ3SWJZLzlIa2pmeld6ZHd1RXpTSzl3QVRjT292dEhXZVpYK1ZXZFVv?= =?utf-8?B?L0cyUHRndUxoSndVMGJDeXpjQjZCZUprVG5DUmY4b3ZXbFVUOW56b3RSODJu?= =?utf-8?B?VjI3Q1oxRmNnOGZaYVdCRUwxY29Ud0NBR0NMUEVKenVFOU5Qcjc0K3pNNGVy?= =?utf-8?B?SzhkYWd5c1hZQUNWOHNnT1NudUx6Z2w2ZjJubEpZTUt4NTFxNEVFL0gwMlVq?= =?utf-8?B?L0d2enNwVkVuN1JkV2FNc0huWWZaejVrenZzSVhoTUh1aE1xMTRRLzlKY3NT?= =?utf-8?B?WEl6OVZ1bmM0ZHJUYzIzRUNkNWt4NVRoRmwvMGEyaG1iYjFiT2c3cUpRYmt6?= =?utf-8?B?VE9zYjZCRGF5dXZLUzdQRExJdjBsQXBya2Y3VFlkL3VUemFEQjU2Mm5tcUht?= =?utf-8?B?cWFNNWJPbm03MEtwOTFMQ1V3aTVFaGhEeUI1M2o1TmdoNXVNZnA0bmdFWjJN?= =?utf-8?B?MjRyUFlCekdvMEowcXBCWWExQSsvYXZEbUV4Qkx5dE5BdUk1R1UveW1RSnJC?= =?utf-8?B?dE1TRjQ2NDcrTXNPSUgxc0lGakFYRUQ0TDE1QlZrTTNBSU9HSzBtUy9Ka083?= =?utf-8?B?aXpIWStoUTlvc1BOL3cwcFJ0emhEMTg0emZSbW54d3FGUTFKbEQ2blQ3T3Zv?= =?utf-8?B?MU1sSStZWGhudWN3ekkvc2tmSFZOWFZ3emFqcWxKY2VyNUpmMnFHT0lDWkFo?= =?utf-8?B?WE5yOGdORWtkQnIvNnBzek0wVThWMTl1c2NrQzFGbVhBS0djbnByV0xVSlpH?= =?utf-8?B?NEhaVVZTYStPSjk2cWkvZjNzTzJkVlRBSGRiRkYvRzYwMWk5aStHRVpUQlNL?= =?utf-8?B?M1puS2QrK1g2TGhBUlhSdHpaVWtMaHg2U25xNUtiNDViUEVjZjVBb3dYM0xJ?= =?utf-8?B?dm9vMHl1L0pLRkE4dDJGVkIwOCtNTTVLV2oyb1JNMFkxR2Rac2tzbUU3OTNt?= =?utf-8?B?MHVSRmlCdFdzOTg1aXhkSEczeXd5ZzNGT1QwTWh2LzB4VlZZSEhEdTI5Nk41?= =?utf-8?B?YWp3YllNTlB4emtoYmlRS2l1aG9QOG11Vzg1UUpZU2hrcGJPM1JIY1JSSFdt?= =?utf-8?B?bTdiS1FxOUZYanNTWUVoU21YanFmS3dxL0lKR1ljS2NqMjF5d0ZpOU1yT1Q5?= =?utf-8?B?L1Ntd3d2VjJ3N081dkVlamhHZ3pOYkNaVXNYTkhORWZVUU13TWwrUHJWK01w?= =?utf-8?B?YzRkMTByWHAybXNWMi9GY1BDd1ZQaWpmTGZDMUdqUzBNL0NJMkRBdHorclFY?= =?utf-8?B?MVFDZmtHb2VCMXl5OEt5WGdpd2x4TFFLckd0dy9NVlNrZjNpcEkzRFBOT3lx?= =?utf-8?B?b3MvZXR6VVlTbmEvUmpaUnVnYW4rbWxjWnVsOFhvUy9NOE5hbmZPZi9kZE82?= =?utf-8?B?MGY0eHhpOXgrSTJFcS9yTTNEdEtNWTNjSnJKaGJSeGpBMWlNRnF4MHRvSXN3?= =?utf-8?B?MHhFRWhxS2hvT1lIamlTOUd4QnRkU3hpUlcvNWR4T0xNelh1ODRSbFQ1QlUw?= =?utf-8?B?TFhjRlEzUG1JeitURGUrTjRvVWNkN2pUd0NEdlJpdmNXRFdLVzZ5cEMrZjho?= =?utf-8?B?VU1janZzbzM3ZHpUSTh1SFQ5Ni9VdVdPOGtYeGJIRWE1a2N2cVh4d21SZVJu?= =?utf-8?B?ZWNvaDFQUXVxYWt2Uy9wZ1N6WUNQY2xPbXNiMFdKZ2tUQjBDSDF5NzdIcHFv?= =?utf-8?B?dkJqUmhHaUQ0WkpDUmNLUTRMbndtcFVvb3VZMjdSRE1NRzJiWFcySm5naUJy?= =?utf-8?B?am9yRDlTT3VOZlFxRTJRY28wd1ZUbXdGWWZWWnl1T1NJd3AvRkYwSUM1SFFs?= =?utf-8?B?bHRxM0RIL0dvR2RkbytvVnlKUnpJenJ3eUxDNnZJNkFNcDcvVlZlOGgwaEpw?= =?utf-8?B?dmJjZ2IvYjh1V29BY1lqOUE5eC8ySWs5RnZDbFFuU0xHNEU2M3BVZHFHcE1J?= =?utf-8?B?TTdhSUZJamtBPT0=?= X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: c997eca3-3522-4e5b-7485-08da3718eb9f X-MS-Exchange-CrossTenant-AuthSource: DB6PR0101MB2214.eurprd01.prod.exchangelabs.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 May 2022 08:49:00.3028 (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: AM9PR01MB7220 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: 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 &. > 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? IMO a better fix for this would be to wrap the macro in a do {} while (0) to keep the macro calls function-like. Anyway, you should have mentioned in the commit message that your aim is to fix this uncommon warning. > 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"). - 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".