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 1739D4E143
for <ffmpegdev@gitmailbox.com>; Mon, 28 Apr 2025 19:56:45 +0000 (UTC)
Received: from [127.0.1.1] (localhost [127.0.0.1])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 03E9D68A112;
Mon, 28 Apr 2025 22:56:39 +0300 (EEST)
Received: from mail-wr1-f41.google.com (mail-wr1-f41.google.com
[209.85.221.41])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id CA675689BC3
for <ffmpeg-devel@ffmpeg.org>; Mon, 28 Apr 2025 22:56:31 +0300 (EEST)
Received: by mail-wr1-f41.google.com with SMTP id
ffacd0b85a97d-3913b539aabso2853898f8f.2
for <ffmpeg-devel@ffmpeg.org>; Mon, 28 Apr 2025 12:56:31 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=gmail.com; s=20230601; t=1745870191; x=1746474991; darn=ffmpeg.org;
h=user-agent:in-reply-to:content-disposition:mime-version:references
:mail-followup-to:message-id:subject:cc:to:from:date:from:to:cc
:subject:date:message-id:reply-to;
bh=PF/oVHGH1s6N4vuOGm4dy/CSdPT5Yd2KH8UMXycgTEE=;
b=QIhvvKf0iiKUw7snRKFxtMXlY9Nk+ToZ+xAeQQZ+ZIGTJYErBGjpO4iNyyLiqc9zW8
VrWrrQfUNt6MPzkVY03HKbta9rA4ot8Wbj5CwiRkSQpyCSEAb9VPIFiYn1cZgs9PWSsr
mt0VjXclxRRHoFVHvnTUb13VyDO4dh4pUnp2odxxnBIVfXQu4gOljNUS8PPSqg6JuX3n
+H5ZffR1ypXayM+4VbYtjyXn76kBNpNztzIj8PAyrCwX/Sw7yYjcWghJtrGpvPusth/s
Tq2a9YDnQBmJy6GOOtOE6cDaGHNYT8VF6LnbZYjYQt7pfLhdL2+OYXa3K/0j75hCfvJ3
/ppQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20230601; t=1745870191; x=1746474991;
h=user-agent:in-reply-to:content-disposition:mime-version:references
:mail-followup-to:message-id:subject:cc:to:from:date
:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;
bh=PF/oVHGH1s6N4vuOGm4dy/CSdPT5Yd2KH8UMXycgTEE=;
b=V3zj4mkgG0hSCZl995WsD9negHEcYgKOWkOyD9b5OfiKYON/KL/RzpqoN3Ux9q2/l9
54gTfDnOGRb5HgiFljZ7xa0UzV/kBjtDCnW5Fu+GDl6zlsztoeJGhWsD9Q5AkJlaeeZC
8f5BeZjtJ9Xn5RO8w7jmoQ3+I5teDyTyijRVS3cGr9VQPJ4Ls2iG0Z5ljcU2CdBYdK73
2ix+oOFkk2vuEOhaC6zhx9iQALC1YBkIl9MZ+dmvI4g0pvzG1Q6EZnczzW2I6qzmq6by
I9g4wu3wxnSoxbQ/rqTA9ggaqg6+cOhflqck1nlytxw5/IS3IPAIZ0x6b05Z5IP/5ex/
91SQ==
X-Gm-Message-State: AOJu0YwgVGtKrO3To8YfkyZhBAzE2+q9A3gQVa9LifjjiLUnu/Svf1wc
dMye/fVj+licxOyKaIPCACpyZX7PR/Gua9hDMt+jH/TXRaDeJUSv+gdh2w==
X-Gm-Gg: ASbGncs9reZs5HOed4n1jBGjOFVX9Yr8J2AErUSWcD9SuO3nZiCkgzUo2MJeitp/n5l
aeDe9blEzOhYsS/lvOSPnEUhJgzBY85cVS9v6ldKrtfGCNUR6GGgeUxsJJLjRMiU8EM2PRqFJsf
8AyadOAwJmpRtpP3wift7QAN3IumKs3U4B17NlLUeVFAfTLfvmQypVK8oMLPTykPvFHUfL5s7bp
P/mDlEPqRbBf4EZ2eUYsfz/NVJ8KRhzRVhdg9mq0VtnWCdrtGFLrDQAE30QJEA9Aa230vTZSVF+
3H4lEiP5uJ3bsOtNWhumNKQrbkmhXFGjxw8hsanOm0c5Z6HRWCIhaHocDKwzPKg5ntiUaXUkVva
BvVrU
X-Google-Smtp-Source: AGHT+IFWCAJWAUK1EyVFjWKxBfkXIOD1V0ogtYHOEgQ5Y7Y00DcNYoSUV3CmBCg4B0JsJDLf4erhiQ==
X-Received: by 2002:a05:6000:40cb:b0:390:f394:6271 with SMTP id
ffacd0b85a97d-3a07ab85dc3mr7868024f8f.43.1745870190462;
Mon, 28 Apr 2025 12:56:30 -0700 (PDT)
Received: from mariano (dynamic-adsl-84-220-189-10.clienti.tiscali.it.
[84.220.189.10]) by smtp.gmail.com with ESMTPSA id
ffacd0b85a97d-3a07dbd6ea1sm7953935f8f.7.2025.04.28.12.56.29
(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
Mon, 28 Apr 2025 12:56:29 -0700 (PDT)
Received: by mariano (Postfix, from userid 1000)
id 91627BFCE8; Mon, 28 Apr 2025 21:56:28 +0200 (CEST)
Date: Mon, 28 Apr 2025 21:56:28 +0200
From: Stefano Sabatini <stefasab@gmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Message-ID: <aA/dbPipiEq0ZPw1@mariano>
Mail-Followup-To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>,
softworkz <softworkz-at-hotmail.com@ffmpeg.org>
References: <pull.66.v6.ffstaging.FFmpeg.1745457192.ffmpegagent@gmail.com>
<pull.66.v7.ffstaging.FFmpeg.1745623868.ffmpegagent@gmail.com>
<1a4044ba238e9cdd13c8db56a344387748f64633.1745623868.git.ffmpegagent@gmail.com>
MIME-Version: 1.0
Content-Disposition: inline
In-Reply-To: <1a4044ba238e9cdd13c8db56a344387748f64633.1745623868.git.ffmpegagent@gmail.com>
User-Agent: Mutt/2.1.4 (2021-12-11)
Subject: Re: [FFmpeg-devel] [PATCH v7 02/13] fftools/textformat: Apply
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>
Cc: softworkz <softworkz@hotmail.com>
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/aA%2FdbPipiEq0ZPw1@mariano/>
List-Archive: <https://master.gitmailbox.com/ffmpegdev/>
List-Post: <mailto:ffmpegdev@gitmailbox.com>
On date Friday 2025-04-25 23:30:57 +0000, softworkz wrote:
> From: softworkz <softworkz@hotmail.com>
>
> Perform multiple improvements to increase code robustness.
> In particular:
> - favor unsigned counters for loops
> - add missing checks
> - avoid possible leaks
> - move variable declarations to inner scopes when feasible
> - provide explicit type-casting when needed
>
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---
> fftools/textformat/avtextformat.c | 85 ++++++++++++++++++++-----------
> fftools/textformat/avtextformat.h | 6 +--
> fftools/textformat/tf_default.c | 8 ++-
> fftools/textformat/tf_ini.c | 2 +-
> fftools/textformat/tf_json.c | 17 ++++---
> fftools/textformat/tf_xml.c | 3 --
> fftools/textformat/tw_avio.c | 11 +++-
> 7 files changed, 83 insertions(+), 49 deletions(-)
>
> diff --git a/fftools/textformat/avtextformat.c b/fftools/textformat/avtextformat.c
> index 74d179c516..1939a1f739 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++)
> av_bprintf(bp, "%02X", ubuf[i]);
> }
>
> @@ -137,6 +136,9 @@ int avtext_context_open(AVTextFormatContext **ptctx, const AVTextFormatter *form
> AVTextFormatContext *tctx;
> int i, ret = 0;
>
> + if (!ptctx || !formatter)
> + return AVERROR(EINVAL);
assert0?
> +
> if (!(tctx = av_mallocz(sizeof(AVTextFormatContext)))) {
> ret = AVERROR(ENOMEM);
> goto fail;
> @@ -209,25 +211,26 @@ int avtext_context_open(AVTextFormatContext **ptctx, const AVTextFormatter *form
> av_log(NULL, AV_LOG_ERROR, " %s", n);
> av_log(NULL, AV_LOG_ERROR, "\n");
> }
> - return ret;
> + goto fail;
> }
>
> /* 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);
> bprint_bytes(&bp, p0, p - p0),
> av_log(tctx, AV_LOG_ERROR,
> "Invalid UTF8 sequence %s found in string validation replace '%s'\n",
> bp.str, tctx->string_validation_replacement);
> - return ret;
> + av_bprint_finalize(&bp, NULL);
I see no advantage in this, since it's adding dynamic allocation and
the need to free which was previously not needed
> + goto fail;
> }
> }
> }
> @@ -255,6 +258,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 (section_id < 0 || section_id >= tctx->nb_sections)
> + return;
Given this is a check on the input data, we should probably make it
fail with a log message, meaning we should also check the return value
from the callee.
Given this is not a public API I'm not against the assert though.
[...]
> int avtextwriter_create_file(AVTextWriterContext **pwctx, const char *output_filename)
> {
> + if (!output_filename || !output_filename[0]) {
> + av_log(NULL, AV_LOG_ERROR, "The output_filename cannot be NULL or empty\n");
> + return AVERROR(EINVAL);
> + }
> +
> IOWriterContext *ctx;
> int ret;
nit here and below: move declarations before actual code for stylistic consistency
[...]
_______________________________________________
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".