From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <ffmpeg-devel-bounces@ffmpeg.org>
Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100])
	by master.gitmailbox.com (Postfix) with ESMTPS id 671754CEB4
	for <ffmpegdev@gitmailbox.com>; Tue, 15 Apr 2025 01:06:04 +0000 (UTC)
Received: from [127.0.1.1] (localhost [127.0.0.1])
	by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 75DBE687BEC;
	Tue, 15 Apr 2025 04:06:00 +0300 (EEST)
Received: from EUR02-DB5-obe.outbound.protection.outlook.com
 (mail-db5eur02olkn2018.outbound.protection.outlook.com [40.92.50.18])
 by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id D52A2687BEC
 for <ffmpeg-devel@ffmpeg.org>; Tue, 15 Apr 2025 04:05:53 +0300 (EEST)
ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none;
 b=EzBGGRwc9ja4Z5P6lcElPUJIgJ8a/KZDRigBgG7JQc6Q8cbMa0E5oOvXCsBqPOJVQhZH/VvmuPrP4I+gvLWP5vA0oC3ItsNEGpJr9+lpA/1KJ7NtfZ+Sc6Z7/5YZEgt3z1GIrmB1QENiwVvCVIGQuMq2yTGCHby8tX1ReFTRzPi44hpeh+d+f6LslGu67gNOfBRexhPkEcvjRn9Hhv5Xor9Qx/VEqSmsIeYE7oCngSLjvUxzODZ4VzNkEB6k6IPLanRvKoLW/h3iOeuCOUCdC9yFp2DSAty9C6RIc8nCtjJI2YzSZgSXP0NbDgwcqSzsF6yq2ifUI3MHdLfbIQ5OeA==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; 
 s=arcselector10001;
 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=/+ByrJ4ZYE9p7OjZhYvscV94Jl10rVMiywpvhBh0n/M=;
 b=iqH9iOP+TknBRF8QEO1kPrWa4Wd6wp4iCibco9kr2+z/AFlo2MwiW5SGlKSjqkUYl/TimuMoTlsxvDrkrUZYmFLyvEKGWu1BbxJ//3M7nJPCX/6LfH2z9uWUgj/XFFLvuv4B0bksu95fWluskXb/YERWPO2uLLGHq92tUmz6Ol4+2pbxzgxQnI56Grvz+Rn2fzo2XxHY3sXrPUlanrh1onwkYE0YdSZjDzFOrjo9BVYHkoiyKyai6zX3KI/qVd89TF3kOX+6IYSQB2cmeXMLsXxHnUySMXY4DF6K3+5WehIZlWEW++RsRZOQ97gvufp9QeYWWetkQD6/b3VVDbqpoQ==
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=/+ByrJ4ZYE9p7OjZhYvscV94Jl10rVMiywpvhBh0n/M=;
 b=eo1OuMhQnIaXc8++wbZgCrYomHHIX1R5l++TL73eFGAmCMtW1Du2rcpNayd1oAbRTBWDOsZjyBBgwb0hFSNaigOdEdJrQ8WSb8WSt1E7X3pnwy6JGCqZpckHDcgsuNB/tkBinbD2SNFLTGLaI5fOqWoN/P7A0aNgyxm83dB69DdYKSEGhTsO81BU8DZJuREc3JRrYMyQ2zYkNVovjaoa/SFf9WKXd6Dx4M3TUb+Yo7iQsyGDOIRDypq1ZWJvDKbDPD6Gxbg83leXuMdPwi6FcHNJu0VkXd9Umjb4Zv+VR0d/4Eq/e05KvYqmD2bN1Q0chPkqjshTghNn20yfSsowQQ==
Received: from GV1P250MB0737.EURP250.PROD.OUTLOOK.COM (2603:10a6:150:8e::17)
 by PA1P250MB1087.EURP250.PROD.OUTLOOK.COM (2603:10a6:102:45d::6) with
 Microsoft SMTP Server (version=TLS1_2,
 cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8655.13; Tue, 15 Apr
 2025 01:05:45 +0000
Received: from GV1P250MB0737.EURP250.PROD.OUTLOOK.COM
 ([fe80::d6a1:e3af:a5f1:b614]) by GV1P250MB0737.EURP250.PROD.OUTLOOK.COM
 ([fe80::d6a1:e3af:a5f1:b614%5]) with mapi id 15.20.8655.012; Tue, 15 Apr 2025
 01:05:45 +0000
Message-ID: <GV1P250MB073708D33D46AB881810F1728FB22@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM>
Date: Tue, 15 Apr 2025 03:05:44 +0200
User-Agent: Mozilla Thunderbird
To: ffmpeg-devel@ffmpeg.org
References: <pull.66.ffstaging.FFmpeg.1744634826.ffmpegagent@gmail.com>
 <04cce9150062afbe63fcf54af0f8480c386f4286.1744634827.git.ffmpegagent@gmail.com>
Content-Language: en-US
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
In-Reply-To: <04cce9150062afbe63fcf54af0f8480c386f4286.1744634827.git.ffmpegagent@gmail.com>
X-ClientProxiedBy: FR4P281CA0280.DEUP281.PROD.OUTLOOK.COM
 (2603:10a6:d10:e6::16) To GV1P250MB0737.EURP250.PROD.OUTLOOK.COM
 (2603:10a6:150:8e::17)
X-Microsoft-Original-Message-ID: <50193696-490f-46b2-bb5b-c473bbab8e9b@outlook.com>
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
X-MS-PublicTrafficType: Email
X-MS-TrafficTypeDiagnostic: GV1P250MB0737:EE_|PA1P250MB1087:EE_
X-MS-Office365-Filtering-Correlation-Id: c4951888-18a5-4d8c-702b-08dd7bb9a6b3
X-Microsoft-Antispam: BCL:0;
 ARA:14566002|461199028|8060799006|6090799003|19110799003|5072599009|15080799006|7092599003|440099028|3412199025|41001999003;
X-Microsoft-Antispam-Message-Info: =?utf-8?B?MHFnUWxuL3VOKzZWUzBoejlrRFZ4MlBja1JOMVplejZ3M0Y3dVorVmF0YnRu?=
 =?utf-8?B?UVFhQkd2dk5iOUNyaktKbkZMMnd0ZUJRSWRiaW9YOExsSXRIT2grek1nK1RQ?=
 =?utf-8?B?dEZaRW1nWlN0VnI5Ni9kdndLa2h6VlRJeWdzbXVqL2hyKy9QcXNjSFlnUVAw?=
 =?utf-8?B?TzRCVFo0ViswZ1BSdFVod2VyMUNOUGx4NXErb245QzlTZENIclJGMFRTL1Y3?=
 =?utf-8?B?aXdYVHJZUUpDUkVabElpZE1rL0NzZHJZVmJFMEZ3RnNQSDIyTFJMRm9TTElI?=
 =?utf-8?B?bktJQzNhR1ppTlF5VXZRVGk3LytrZXkwd2JqUTVrMUMreThUMzZnVzFNY0xm?=
 =?utf-8?B?QVkxV0cwZTh2QnNCR3h1djFGa0Z2SkVaZzRHenZZQzd1bVZKdDBoS0dZRHow?=
 =?utf-8?B?TmpDTzh2cXpKQUptRzlTOTlSUzVEVW1JV1ZsbXVZaEh0REdKYkltcExEK1p5?=
 =?utf-8?B?NzErUHA3a21xSXBuRTRFejdBV0xFdzNnR1VpNkYrWGZFUjRNNkdhZ3lIaWFY?=
 =?utf-8?B?V0RaWEZqWjJ0WFA4NG1CQ2hNRXU1U0RPUVdrQ1JVWDFUUDIycytRSUdxVmwx?=
 =?utf-8?B?NllYdVU1UGxCSHlKRzFHK0JydllNWHhJOWFLTlF4VE5ZeWRBU3ZNZDh1dlZB?=
 =?utf-8?B?WHFFd21yM1RTWVdybDBsOUdmSFdjazk0TmdQQWdRcFEwUURQaVJFWC9XTmJT?=
 =?utf-8?B?MEo3UTk2ZmZFc1NVczJrRW41aU96Z3RpSURiMFpPU1VUSUJvQjczeEpaWkFm?=
 =?utf-8?B?MXdvUGUxRUlZUkgwMUdoc3hiWnFLb0FkYjJwNExIVXlGbmp4TzNPaFQwNVpY?=
 =?utf-8?B?VDRYUHRmMVdncmY5S3l2R0dvM2syQ2kzMEE0R2pnMm0wNDZpVWdVTzJLYUVZ?=
 =?utf-8?B?bWcvZEY1eHlQMVBpamF2a1NKcEZ6VHRBeFc0U0RkRVMvRGtKQ0dnS2ZPL0RX?=
 =?utf-8?B?U3RuZExaenExaE9LNmlGR1loaElVVUpMcm9NSXBMdmg4bHJmMVRSWkxVMjl2?=
 =?utf-8?B?SU1pa1dzWDIwSFZ5bThSOEk3RHhDenBnRU5EMHVHclAyaFdOVjVPTmlNQXJm?=
 =?utf-8?B?Tlh0eGl5dnRvYkZUS3RLKzF3cDlHcVJWWlJyeHduMkJ1bm13dDRwbS9vaUVw?=
 =?utf-8?B?TmxuZ2JQbWtudFZYSStHdGRVMEduR2lnVXovc1hFT21qZDhpL0FQSXVxYmt4?=
 =?utf-8?B?ejFnbGNuazFiK1JxQTlqNDNFSk9CUmJwSFlReFdWVWRIOU1ya1JLZVlkSmpv?=
 =?utf-8?B?bm5DRHU0WVUveE54TEVtL0F2M2paVWYzVjhzRXovOGxHeE5UQnd6L0NUZWxQ?=
 =?utf-8?B?ZnBKckgrWnBxeEN6Z081cVNLQkYrajVmaE1mZTJnRmRMZVJmVzBUd1Zsb3pU?=
 =?utf-8?B?eGJ1bU5xZVhGZm02QzVjVTBrcHZaeTZMVUFyQVA3WEJDQUpKREpZVXdPT2hD?=
 =?utf-8?B?dEFmb1F3WGFXWFZMcXoyRGxrQ3FQbzhVY0RYUEFtb0wzTXZMVUhxRkkzTTdV?=
 =?utf-8?B?QlFLMzk2SzFyZ2M4UTVYeS9iREt4d1hiT1RxemJjM0kwSXk1RnRBeFE1Slh3?=
 =?utf-8?Q?soBlMvdWaR0xbw7DeTvB+fjtc=3D?=
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?U25rcGI3ZnZNU2h4VW1wTWVNOVhaSnN5cHFxWStJWkErOTVHaEVWQ2lIbnRP?=
 =?utf-8?B?L3JiaFFLYVhDVmgyemYvUkRrb2lIOXg4azhQY3B6bSsxR2IxajNEOFQ5ekVN?=
 =?utf-8?B?N1M5bzVhdmp4QXR3TVk0bVRvdklGV2o2anQwZUpSOE54UkpSWG1ESEpRbENp?=
 =?utf-8?B?V1BDRS9wbThuZDYzQlRaeEwyWSt6MStoSjI0bk5OcFdsRlpuanV0NDZRTlZW?=
 =?utf-8?B?dFFZclZJTFFEMEJQMDAwZGFIRDVROVdKMXJqTW1QRWxwaXNvRUU1ajQ4UGJ2?=
 =?utf-8?B?NDl6WDlpaFNhcDlRRlFNMzhjMU5JUXFIaHIrTUFuTSs2TkNpalR3RCt3Wm1o?=
 =?utf-8?B?QVhPMzJxeVBiQjNDVVRoMWJseUZseTlSUXljWXRmOHFXQ2M5b0c0R2JrYjk4?=
 =?utf-8?B?NkhzeG55bEcwMWNvYkc0ZitVaDZ5czVwS0R4ZmpsNmxFRHRYeDlvTmZUb1NL?=
 =?utf-8?B?MENQMEIvWjdESnpZZU5XMnhZL25VWUZJdzBsdENNejZJRnhPQ291TUhDNFUy?=
 =?utf-8?B?MkszZjJEMWdhaWJXdURuU0w3SnNuNFhTTXU0NEZqWG5GczZIRUhFdzY5SWRD?=
 =?utf-8?B?eU9MUFBLMzVPbmFIMHU0QzRGV0dZRjdjWnpyZm5MY3FKclNsNlJaOE9IdklP?=
 =?utf-8?B?Y2tReC83TmtVUXZ1MFZOVGtXNnExeEtNYkNCU2J6RHd2SG9uME5tYVFUWG9Q?=
 =?utf-8?B?ZGJGdGFab3JCbTBlcjNDcTI3R0pyemlYT2dzU0NkTnJIK01nWklUMEhSbDVx?=
 =?utf-8?B?QTJBc2J3WXh1M3ZBeFRNRXdaalhuMXo1UjB2MEVSbFJpV2RKb3V2UXdPcTFD?=
 =?utf-8?B?a295SGNrSzc5R1ROcW1qa0lwNGY2TnBOUit1YjFqMnZrN0l0QlREZ3RrL0ta?=
 =?utf-8?B?NzYzTEh4cisyM0NFK2YycUl2blAzZUhwci9hV0xqOUE0SHMyZHM3OUNzZDMy?=
 =?utf-8?B?TFNkcFpxS28rZlRMYWlSVjR3VWc0WVJVdVZjK0M4VVpTZ3dZcDNjK3RyMmdy?=
 =?utf-8?B?RWV5UFA3YzFkbW9tTXVRdHpDTnB1aWVRWmMrNFZENGdlZnBpOW4zc0hYdFFX?=
 =?utf-8?B?eGc4NG14dThkQTEzMUQ4YkxKT0dSVUpJdks1NFY1VDNDN1gvZzNpMXdZY2lF?=
 =?utf-8?B?b0hzTngxTC9NNm5HM1A4VWw3c3cwck9uanRXdnZtYmNiYkp5L1JDc3N4am5l?=
 =?utf-8?B?Q1UyV2xtbWJkWS9NMlpWczFrZlY0eWZtRmdyd2xueEFYbG5tRjNtQjA0N3Js?=
 =?utf-8?B?ckpkR2xiQlN6R0tNZzdTR0djbXVORjRqY3VQSFRGN1RGTDZPYzBoODNQT09r?=
 =?utf-8?B?c3ZkYmNNa3FaeWsrbmIzR3pEaDdHcHpESytSSkdKd295ZCtHdHJLRU84b0xl?=
 =?utf-8?B?d3lrS1hWenhtNTZmNEdKNXZjV3cybThxWXNrbC9xeE1pV0hXbDMxaS9OQmQv?=
 =?utf-8?B?VXdHMHQ3S1VWaHZFR2JBdk5LNVNSY2M4L2h3K29rV1JNOUgxQjNNRHM2Rms0?=
 =?utf-8?B?YjN0Ym14VjA0NzhtOElJUmMreW0zbS9WOWxWQkxDZ240VDlJL3JOdUhzRHFV?=
 =?utf-8?B?Um1leE1hZERhNTJHdFR4am5KeDB0dDMvOFFPdlpTQ0Z3RUMxbndxYjlYY3RT?=
 =?utf-8?B?NVJON1hZeDVqaCtUdzNKTGlwYXR1dkpScGxkcVFmTVE3TkJhazFOOWVvUVpO?=
 =?utf-8?B?NXgzWE9uNWx0cXJlUWliQzE2YkdCZE5iZXZ0KytCdUVPVjh0VjYxYjM2WWo5?=
 =?utf-8?Q?AlkIZXC6zebyPlOX58c3OeN7PgV6DxBeJuQDsen?=
X-OriginatorOrg: outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c4951888-18a5-4d8c-702b-08dd7bb9a6b3
X-MS-Exchange-CrossTenant-AuthSource: GV1P250MB0737.EURP250.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Apr 2025 01:05:45.5443 (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: PA1P250MB1087
Subject: Re: [FFmpeg-devel] [PATCH 2/9] fftools/textformat: Quality
 improvements
X-BeenThere: ffmpeg-devel@ffmpeg.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: FFmpeg development discussions and patches <ffmpeg-devel.ffmpeg.org>
List-Unsubscribe: <https://ffmpeg.org/mailman/options/ffmpeg-devel>,
 <mailto:ffmpeg-devel-request@ffmpeg.org?subject=unsubscribe>
List-Archive: <https://ffmpeg.org/pipermail/ffmpeg-devel>
List-Post: <mailto:ffmpeg-devel@ffmpeg.org>
List-Help: <mailto:ffmpeg-devel-request@ffmpeg.org?subject=help>
List-Subscribe: <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>,
 <mailto:ffmpeg-devel-request@ffmpeg.org?subject=subscribe>
Reply-To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: ffmpeg-devel-bounces@ffmpeg.org
Sender: "ffmpeg-devel" <ffmpeg-devel-bounces@ffmpeg.org>
Archived-At: <https://master.gitmailbox.com/ffmpegdev/GV1P250MB073708D33D46AB881810F1728FB22@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM/>
List-Archive: <https://master.gitmailbox.com/ffmpegdev/>
List-Post: <mailto:ffmpegdev@gitmailbox.com>

softworkz:
> From: softworkz <softworkz@hotmail.com>
> 
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---
>  fftools/textformat/avtextformat.c | 121 +++++++++++++++++++-----------
>  fftools/textformat/avtextformat.h |   6 +-
>  fftools/textformat/tf_default.c   |   8 +-
>  fftools/textformat/tf_ini.c       |   2 +-
>  fftools/textformat/tf_json.c      |   8 +-
>  fftools/textformat/tf_xml.c       |   3 -
>  fftools/textformat/tw_avio.c      |   9 ++-
>  7 files changed, 101 insertions(+), 56 deletions(-)
> 
> diff --git a/fftools/textformat/avtextformat.c b/fftools/textformat/avtextformat.c
> index 1ce51d11e2..406025d19d 100644
> --- a/fftools/textformat/avtextformat.c
> +++ b/fftools/textformat/avtextformat.c
> @@ -93,9 +93,8 @@ static const AVClass textcontext_class = {
>  
>  static void bprint_bytes(AVBPrint *bp, const uint8_t *ubuf, size_t ubuf_size)
>  {
> -    int i;
>      av_bprintf(bp, "0X");
> -    for (i = 0; i < ubuf_size; i++)
> +    for (unsigned i = 0; i < ubuf_size; i++)

Why not size_t?

>          av_bprintf(bp, "%02X", ubuf[i]);
>  }
>  
> @@ -110,8 +109,6 @@ int avtext_context_close(AVTextFormatContext **ptctx)
>  
>      av_hash_freep(&tctx->hash);
>  
> -    av_hash_freep(&tctx->hash);
> -
>      if (tctx->formatter->uninit)
>          tctx->formatter->uninit(tctx);
>      for (i = 0; i < SECTION_MAX_NB_LEVELS; i++)
> @@ -141,12 +138,18 @@ int avtext_context_open(AVTextFormatContext      **ptctx,
>      AVTextFormatContext *tctx;
>      int i, ret = 0;
>  
> -    if (!(tctx = av_mallocz(sizeof(AVTextFormatContext)))) {
> +    if (!ptctx || !formatter)
> +        return AVERROR(EINVAL);

Can this happen?

> +
> +    if (!formatter->priv_size && formatter->priv_class)
> +        return AVERROR(EINVAL);

Stuff like this should never happen and should not be checked (or
actually: the proper place to check stuff like this is in test tools
like lavc/tests/avcodec.c, but I don't think it is worth it for fftools).

> +
> +    if (!((tctx = av_mallocz(sizeof(AVTextFormatContext))))) {
>          ret = AVERROR(ENOMEM);
>          goto fail;
>      }
>  
> -    if (!(tctx->priv = av_mallocz(formatter->priv_size))) {
> +    if (formatter->priv_size && !((tctx->priv = av_mallocz(formatter->priv_size)))) {
>          ret = AVERROR(ENOMEM);
>          goto fail;
>      }
> @@ -215,15 +218,15 @@ int avtext_context_open(AVTextFormatContext      **ptctx,
>  
>      /* validate replace string */
>      {
> -        const uint8_t *p = tctx->string_validation_replacement;
> -        const uint8_t *endp = p + strlen(p);
> +        const uint8_t *p = (uint8_t *)tctx->string_validation_replacement;
> +        const uint8_t *endp = p + strlen((const char *)p);
>          while (*p) {
>              const uint8_t *p0 = p;
>              int32_t code;
>              ret = av_utf8_decode(&code, &p, endp, tctx->string_validation_utf8_flags);
>              if (ret < 0) {
>                  AVBPrint bp;
> -                av_bprint_init(&bp, 0, AV_BPRINT_SIZE_AUTOMATIC);
> +                av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED);

This adds a memleak on data where it makes a difference.

>                  bprint_bytes(&bp, p0, p - p0),
>                      av_log(tctx, AV_LOG_ERROR,
>                             "Invalid UTF8 sequence %s found in string validation replace '%s'\n",
> @@ -259,6 +262,9 @@ static const char unit_bit_per_second_str[] = "bit/s";
>  
>  void avtext_print_section_header(AVTextFormatContext *tctx, const void *data, int section_id)
>  {
> +    if (!tctx || section_id < 0 || section_id >= tctx->nb_sections)
> +        return;

Can this happen?

> +
>      tctx->level++;
>      av_assert0(tctx->level < SECTION_MAX_NB_LEVELS);
>  
> @@ -272,6 +278,9 @@ void avtext_print_section_header(AVTextFormatContext *tctx, const void *data, in
>  
>  void avtext_print_section_footer(AVTextFormatContext *tctx)
>  {
> +    if (!tctx || tctx->level < 0 || tctx->level >= SECTION_MAX_NB_LEVELS)
> +        return;

Can this happen?

> +
>      int section_id = tctx->section[tctx->level]->id;
>      int parent_section_id = tctx->level
>          ? tctx->section[tctx->level - 1]->id
> @@ -289,7 +298,12 @@ void avtext_print_section_footer(AVTextFormatContext *tctx)
>  
>  void avtext_print_integer(AVTextFormatContext *tctx, const char *key, int64_t val)
>  {
> -    const struct AVTextFormatSection *section = tctx->section[tctx->level];
> +    const AVTextFormatSection *section;
> +
> +    if (!tctx || !key || tctx->level < 0 || tctx->level >= SECTION_MAX_NB_LEVELS)
> +        return;

Can this happen?

> +
> +    section = tctx->section[tctx->level];
>  
>      if (section->show_all_entries || av_dict_get(section->entries_to_show, key, NULL, 0)) {
>          tctx->formatter->print_integer(tctx, key, val);
> @@ -299,24 +313,28 @@ void avtext_print_integer(AVTextFormatContext *tctx, const char *key, int64_t va
>  
>  static inline int validate_string(AVTextFormatContext *tctx, char **dstp, const char *src)
>  {
> -    const uint8_t *p, *endp;
> +    const uint8_t *p, *endp, *srcp = (const uint8_t *)src;
>      AVBPrint dstbuf;
> +    AVBPrint bp;
>      int invalid_chars_nb = 0, ret = 0;
>  
> +    if (!tctx || !dstp || !src)
> +        return AVERROR(EINVAL);
> +

Can this happen?

> +    *dstp = NULL;
>      av_bprint_init(&dstbuf, 0, AV_BPRINT_SIZE_UNLIMITED);
> +    av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED);
>  
> -    endp = src + strlen(src);
> -    for (p = src; *p;) {
> -        uint32_t code;
> +    endp = srcp + strlen(src);
> +    for (p = srcp; *p;) {
> +        int32_t code;
>          int invalid = 0;
>          const uint8_t *p0 = p;
>  
>          if (av_utf8_decode(&code, &p, endp, tctx->string_validation_utf8_flags) < 0) {
> -            AVBPrint bp;
> -            av_bprint_init(&bp, 0, AV_BPRINT_SIZE_AUTOMATIC);
> -            bprint_bytes(&bp, p0, p-p0);
> -            av_log(tctx, AV_LOG_DEBUG,
> -                   "Invalid UTF-8 sequence %s found in string '%s'\n", bp.str, src);
> +            av_bprint_clear(&bp);
> +            bprint_bytes(&bp, p0, p - p0);
> +            av_log(tctx, AV_LOG_DEBUG, "Invalid UTF-8 sequence %s found in string '%s'\n", bp.str, src);
>              invalid = 1;
>          }
>  
> @@ -336,7 +354,7 @@ static inline int validate_string(AVTextFormatContext *tctx, char **dstp, const
>          }
>  
>          if (!invalid || tctx->string_validation == AV_TEXTFORMAT_STRING_VALIDATION_IGNORE)
> -            av_bprint_append_data(&dstbuf, p0, p-p0);
> +            av_bprint_append_data(&dstbuf, (const char *)p0, p - p0);
>      }
>  
>      if (invalid_chars_nb && tctx->string_validation == AV_TEXTFORMAT_STRING_VALIDATION_REPLACE)
> @@ -346,6 +364,7 @@ static inline int validate_string(AVTextFormatContext *tctx, char **dstp, const
>  
>  end:
>      av_bprint_finalize(&dstbuf, dstp);
> +    av_bprint_finalize(&bp, NULL);
>      return ret;
>  }
>  
> @@ -358,17 +377,18 @@ struct unit_value {
>      const char *unit;
>  };
>  
> -static char *value_string(AVTextFormatContext *tctx, char *buf, int buf_size, struct unit_value uv)
> +static char *value_string(const AVTextFormatContext *tctx, char *buf, int buf_size, struct unit_value uv)
>  {
>      double vald;
> -    int64_t vali;
> +    int64_t vali = 0;
>      int show_float = 0;
>  
>      if (uv.unit == unit_second_str) {
>          vald = uv.val.d;
>          show_float = 1;
>      } else {
> -        vald = vali = uv.val.i;
> +        vald = (double)uv.val.i;
> +        vali = uv.val.i;
>      }
>  
>      if (uv.unit == unit_second_str && tctx->use_value_sexagesimal_format) {
> @@ -387,17 +407,17 @@ static char *value_string(AVTextFormatContext *tctx, char *buf, int buf_size, st
>              int64_t index;
>  
>              if (uv.unit == unit_byte_str && tctx->use_byte_value_binary_prefix) {
> -                index = (int64_t) (log2(vald)) / 10;
> -                index = av_clip(index, 0, FF_ARRAY_ELEMS(si_prefixes) - 1);
> +                index = (int64_t)(log2(vald) / 10);
> +                index = av_clip64(index, 0, FF_ARRAY_ELEMS(si_prefixes) - 1);
>                  vald /= si_prefixes[index].bin_val;
>                  prefix_string = si_prefixes[index].bin_str;
>              } else {
> -                index = (int64_t) (log10(vald)) / 3;
> -                index = av_clip(index, 0, FF_ARRAY_ELEMS(si_prefixes) - 1);
> +                index = (int64_t)(log10(vald) / 3);
> +                index = av_clip64(index, 0, FF_ARRAY_ELEMS(si_prefixes) - 1);
>                  vald /= si_prefixes[index].dec_val;
>                  prefix_string = si_prefixes[index].dec_str;
>              }
> -            vali = vald;
> +            vali = (int64_t)vald;
>          }
>  
>          if (show_float || (tctx->use_value_prefix && vald != (int64_t)vald))
> @@ -425,9 +445,14 @@ void avtext_print_unit_int(AVTextFormatContext *tctx, const char *key, int value
>  
>  int avtext_print_string(AVTextFormatContext *tctx, const char *key, const char *val, int flags)
>  {
> -    const struct AVTextFormatSection *section = tctx->section[tctx->level];
> +    const AVTextFormatSection *section;
>      int ret = 0;
>  
> +    if (!tctx || !key || !val || tctx->level < 0 || tctx->level >= SECTION_MAX_NB_LEVELS)
> +        return AVERROR(EINVAL);

Can this happen?

> +
> +    section = tctx->section[tctx->level];
> +
>      if (tctx->show_optional_fields == SHOW_OPTIONAL_FIELDS_NEVER ||
>          (tctx->show_optional_fields == SHOW_OPTIONAL_FIELDS_AUTO
>              && (flags & AV_TEXTFORMAT_PRINT_STRING_OPTIONAL)
> @@ -462,7 +487,7 @@ int avtext_print_string(AVTextFormatContext *tctx, const char *key, const char *
>  void avtext_print_rational(AVTextFormatContext *tctx, const char *key, AVRational q, char sep)
>  {
>      AVBPrint buf;
> -    av_bprint_init(&buf, 0, AV_BPRINT_SIZE_AUTOMATIC);
> +    av_bprint_init(&buf, 0, AV_BPRINT_SIZE_UNLIMITED);

This is strictly worse than what was here before: With UNLIMITED you
would have a memleak in case the internal buffer wouldn't suffice.
(But anyway, this should use snprintf. I just sent a patch for this.)

>      av_bprintf(&buf, "%d%c%d", q.num, sep, q.den);
>      avtext_print_string(tctx, key, buf.str, 0);
>  }
> @@ -470,12 +495,11 @@ void avtext_print_rational(AVTextFormatContext *tctx, const char *key, AVRationa
>  void avtext_print_time(AVTextFormatContext *tctx, const char *key,
>                         int64_t ts, const AVRational *time_base, int is_duration)
>  {
> -    char buf[128];
> -
>      if ((!is_duration && ts == AV_NOPTS_VALUE) || (is_duration && ts == 0)) {
>          avtext_print_string(tctx, key, "N/A", AV_TEXTFORMAT_PRINT_STRING_OPTIONAL);
>      } else {
> -        double d = ts * av_q2d(*time_base);
> +        char buf[128];
> +        double d = av_q2d(*time_base) * (double)ts;

We actually try to avoid explicit casts where possible.

>          struct unit_value uv;
>          uv.val.d = d;
>          uv.unit = unit_second_str;
> @@ -496,7 +520,8 @@ void avtext_print_data(AVTextFormatContext *tctx, const char *name,
>                         const uint8_t *data, int size)
>  {
>      AVBPrint bp;
> -    int offset = 0, l, i;
> +    unsigned offset = 0;
> +    int l, i;
>  
>      av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED);
>      av_bprintf(&bp, "\n");
> @@ -523,25 +548,29 @@ void avtext_print_data(AVTextFormatContext *tctx, const char *name,
>  void avtext_print_data_hash(AVTextFormatContext *tctx, const char *name,
>                              const uint8_t *data, int size)
>  {
> -    char *p, buf[AV_HASH_MAX_SIZE * 2 + 64] = { 0 };
> +    char buf[AV_HASH_MAX_SIZE * 2 + 64] = { 0 };
> +    int len;
>  
>      if (!tctx->hash)
>          return;
>  
>      av_hash_init(tctx->hash);
>      av_hash_update(tctx->hash, data, size);
> -    snprintf(buf, sizeof(buf), "%s:", av_hash_get_name(tctx->hash));
> -    p = buf + strlen(buf);
> -    av_hash_final_hex(tctx->hash, p, buf + sizeof(buf) - p);
> +    len = snprintf(buf, sizeof(buf), "%s:", av_hash_get_name(tctx->hash));
> +    av_hash_final_hex(tctx->hash, (uint8_t *)&buf[len], (int)sizeof(buf) - len);

Is it guaranteed that the output of snprintf() is not truncated?

>      avtext_print_string(tctx, name, buf, 0);
>  }
>  
>  void avtext_print_integers(AVTextFormatContext *tctx, const char *name,
> -                                  uint8_t *data, int size, const char *format,
> -                                  int columns, int bytes, int offset_add)
> +                           uint8_t *data, int size, const char *format,
> +                           int columns, int bytes, int offset_add)
>  {
>      AVBPrint bp;
> -    int offset = 0, l, i;
> +    unsigned offset = 0;
> +    int l, i;
> +
> +    if (!name || !data || !format || columns <= 0 || bytes <= 0)
> +        return;

Can this happen?

>  
>      av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED);
>      av_bprintf(&bp, "\n");
> @@ -607,12 +636,18 @@ int avtextwriter_context_open(AVTextWriterContext **pwctx, const AVTextWriter *w
>      AVTextWriterContext *wctx;
>      int ret = 0;
>  
> -    if (!(wctx = av_mallocz(sizeof(AVTextWriterContext)))) {
> +    if (!pwctx || !writer)
> +        return AVERROR(EINVAL);
> +
> +    if (!writer->priv_size && writer->priv_class)

Stuff like this should never happen and should therefore not be checked.

> +        return AVERROR(EINVAL);
> +
> +    if (!((wctx = av_mallocz(sizeof(AVTextWriterContext))))) {
>          ret = AVERROR(ENOMEM);
>          goto fail;
>      }
>  
> -    if (!(wctx->priv = av_mallocz(writer->priv_size))) {
> +    if (writer->priv_size && !((wctx->priv = av_mallocz(writer->priv_size)))) {
>          ret = AVERROR(ENOMEM);
>          goto fail;
>      }
> diff --git a/fftools/textformat/avtextformat.h b/fftools/textformat/avtextformat.h
> index 03564d14a7..e519094f4f 100644
> --- a/fftools/textformat/avtextformat.h
> +++ b/fftools/textformat/avtextformat.h
> @@ -21,9 +21,7 @@
>  #ifndef FFTOOLS_TEXTFORMAT_AVTEXTFORMAT_H
>  #define FFTOOLS_TEXTFORMAT_AVTEXTFORMAT_H
>  
> -#include <stddef.h>
>  #include <stdint.h>
> -#include "libavutil/attributes.h"
>  #include "libavutil/dict.h"
>  #include "libavformat/avio.h"
>  #include "libavutil/bprint.h"
> @@ -103,7 +101,7 @@ struct AVTextFormatContext {
>      unsigned int nb_item_type[SECTION_MAX_NB_LEVELS][SECTION_MAX_NB_SECTIONS];
>  
>      /** section per each level */
> -    const struct AVTextFormatSection *section[SECTION_MAX_NB_LEVELS];
> +    const AVTextFormatSection *section[SECTION_MAX_NB_LEVELS];
>      AVBPrint section_pbuf[SECTION_MAX_NB_LEVELS]; ///< generic print buffer dedicated to each section,
>                                                    ///  used by various formatters
>  
> @@ -124,7 +122,7 @@ struct AVTextFormatContext {
>  #define AV_TEXTFORMAT_PRINT_STRING_VALIDATE 2
>  
>  int avtext_context_open(AVTextFormatContext **ptctx, const AVTextFormatter *formatter, AVTextWriterContext *writer_context, const char *args,
> -                        const struct AVTextFormatSection *sections, int nb_sections,
> +                        const AVTextFormatSection *sections, int nb_sections,
>                          int show_value_unit,
>                          int use_value_prefix,
>                          int use_byte_value_binary_prefix,
> diff --git a/fftools/textformat/tf_default.c b/fftools/textformat/tf_default.c
> index 14ef9fe8f9..3b05d25f36 100644
> --- a/fftools/textformat/tf_default.c
> +++ b/fftools/textformat/tf_default.c
> @@ -70,9 +70,10 @@ DEFINE_FORMATTER_CLASS(default);
>  /* lame uppercasing routine, assumes the string is lower case ASCII */
>  static inline char *upcase_string(char *dst, size_t dst_size, const char *src)
>  {
> -    int i;
> +    unsigned i;
> +

Why not size_t?

>      for (i = 0; src[i] && i < dst_size - 1; i++)
> -        dst[i] = av_toupper(src[i]);
> +        dst[i] = (char)av_toupper(src[i]);
>      dst[i] = 0;
>      return dst;
>  }
> @@ -108,6 +109,9 @@ static void default_print_section_footer(AVTextFormatContext *wctx)
>      const struct AVTextFormatSection *section = wctx->section[wctx->level];
>      char buf[32];
>  
> +    if (!section)
> +        return;

Can this happen?

> +
>      if (def->noprint_wrappers || def->nested_section[wctx->level])
>          return;
>  
> diff --git a/fftools/textformat/tf_ini.c b/fftools/textformat/tf_ini.c
> index 9e1aa60e09..ec471fd480 100644
> --- a/fftools/textformat/tf_ini.c
> +++ b/fftools/textformat/tf_ini.c
> @@ -92,7 +92,7 @@ static char *ini_escape_str(AVBPrint *dst, const char *src)
>              /* fallthrough */
>          default:
>              if ((unsigned char)c < 32)
> -                av_bprintf(dst, "\\x00%02x", c & 0xff);
> +                av_bprintf(dst, "\\x00%02x", (unsigned char)c);
>              else
>                  av_bprint_chars(dst, c, 1);
>              break;
> diff --git a/fftools/textformat/tf_json.c b/fftools/textformat/tf_json.c
> index 24838b35ec..f286838d3c 100644
> --- a/fftools/textformat/tf_json.c
> +++ b/fftools/textformat/tf_json.c
> @@ -82,13 +82,18 @@ static const char *json_escape_str(AVBPrint *dst, const char *src, void *log_ctx
>      static const char json_subst[]  = { '"', '\\',  'b',  'f',  'n',  'r',  't', 0 };
>      const char *p;
>  
> +    if (!src) {
> +        av_log(log_ctx, AV_LOG_ERROR, "json_escape_str: NULL source string\n");
> +        return NULL;
> +    }

Can this even happen?

> +
>      for (p = src; *p; p++) {
>          char *s = strchr(json_escape, *p);
>          if (s) {
>              av_bprint_chars(dst, '\\', 1);
>              av_bprint_chars(dst, json_subst[s - json_escape], 1);
>          } else if ((unsigned char)*p < 32) {
> -            av_bprintf(dst, "\\u00%02x", *p & 0xff);
> +            av_bprintf(dst, "\\u00%02x", (unsigned char)*p);
>          } else {
>              av_bprint_chars(dst, *p, 1);
>          }
> @@ -107,6 +112,7 @@ static void json_print_section_header(AVTextFormatContext *wctx, const void *dat
>          wctx->section[wctx->level-1] : NULL;
>  
>      if (wctx->level && wctx->nb_item[wctx->level-1])
> +    if (wctx->level && wctx->nb_item[wctx->level - 1])
>          writer_put_str(wctx, ",\n");
>  
>      if (section->flags & AV_TEXTFORMAT_SECTION_FLAG_IS_WRAPPER) {
> diff --git a/fftools/textformat/tf_xml.c b/fftools/textformat/tf_xml.c
> index 76271dbaa6..eceeda81e5 100644
> --- a/fftools/textformat/tf_xml.c
> +++ b/fftools/textformat/tf_xml.c
> @@ -18,10 +18,7 @@
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>   */
>  
> -#include <limits.h>
> -#include <stdarg.h>
>  #include <stdint.h>
> -#include <stdio.h>
>  #include <string.h>
>  
>  #include "avtextformat.h"
> diff --git a/fftools/textformat/tw_avio.c b/fftools/textformat/tw_avio.c
> index d335d35a56..3c7492aa06 100644
> --- a/fftools/textformat/tw_avio.c
> +++ b/fftools/textformat/tw_avio.c
> @@ -63,7 +63,7 @@ static void io_w8(AVTextWriterContext *wctx, int b)
>  static void io_put_str(AVTextWriterContext *wctx, const char *str)
>  {
>      IOWriterContext *ctx = wctx->priv;
> -    avio_write(ctx->avio_context, str, strlen(str));
> +    avio_write(ctx->avio_context, (const unsigned char *)str, (int)strlen(str));
>  }
>  
>  static void io_printf(AVTextWriterContext *wctx, const char *fmt, ...)
> @@ -89,10 +89,12 @@ const AVTextWriter avtextwriter_avio = {
>  
>  int avtextwriter_create_file(AVTextWriterContext **pwctx, const char *output_filename, int close_on_uninit)
>  {
> +    if (!pwctx || !output_filename || !output_filename[0])
> +        return AVERROR(EINVAL);

Can this happen?

> +
>      IOWriterContext *ctx;
>      int ret;
>  
> -
>      ret = avtextwriter_context_open(pwctx, &avtextwriter_avio);
>      if (ret < 0)
>          return ret;
> @@ -114,6 +116,9 @@ int avtextwriter_create_file(AVTextWriterContext **pwctx, const char *output_fil
>  
>  int avtextwriter_create_avio(AVTextWriterContext **pwctx, AVIOContext *avio_ctx, int close_on_uninit)
>  {
> +    if (!pwctx || !avio_ctx)
> +        return AVERROR(EINVAL);
> +
>      IOWriterContext *ctx;
>      int ret;
>  

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