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 8415C4BBBD
for <ffmpegdev@gitmailbox.com>; Mon, 28 Apr 2025 20:24:31 +0000 (UTC)
Received: from [127.0.1.1] (localhost [127.0.0.1])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 79595687D1A;
Mon, 28 Apr 2025 23:24:27 +0300 (EEST)
Received: from mail-wr1-f51.google.com (mail-wr1-f51.google.com
[209.85.221.51])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 36CFC6804A1
for <ffmpeg-devel@ffmpeg.org>; Mon, 28 Apr 2025 23:24:21 +0300 (EEST)
Received: by mail-wr1-f51.google.com with SMTP id
ffacd0b85a97d-39c31e4c3e5so3268879f8f.0
for <ffmpeg-devel@ffmpeg.org>; Mon, 28 Apr 2025 13:24:21 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=gmail.com; s=20230601; t=1745871860; x=1746476660; 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=nTKbtarAU/fqH9eY/spYzKzGnJGL6+qhwWgmShu1FbA=;
b=m7Bz6cVSaKG16vf8gaOX5Y3Kak/0tQ46GKlSQEurX75M6dm4U/xuDjD/cocLCoQFNb
I9WaNS/xa0CGwOi9e0ooN2TDxt56c0LiHB/bl0DWLG5nMzknehjOriFUhl2S/CapF+r9
wbTCwO1s+sKjIdPBj6nIne0k27x2r11XLimIgSjSkHf40JjC7nJAGvvV6p4t/GNH0l3+
6+A/wM4U/jXl5la+6J1wLZElznXVJn7B0G74GRIv4pGe5b4Kml92zjoqSic1i1kjnAKR
4UkSQEBj2nkZesN3qXJBvzAR+wD995oQpsAyhdsin1dM8OoYpifYTOfi2Sm04SNsb6Xv
Hb+g==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20230601; t=1745871860; x=1746476660;
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=nTKbtarAU/fqH9eY/spYzKzGnJGL6+qhwWgmShu1FbA=;
b=Z7WK50amq9t7eVwGii3PI5qFzip5fDZfq1bL203iX4liJAIPu/gqjbdmPqOgwtSzWb
HwuEFoarJOYoiyH0narIX4wyltOQpTsuBSh1IgePoEbO655hHvF3af3CJOEzCY7iVmwZ
rpSDc1Sf03cnDHrVl5Ro7Fih0AHml1Y8a/2Dpq1IWNzu4Rks3ojtpUj6u2d9vaRVx1uM
IOtO6yz7jS6NQg5NEPzAMFUTSFPGxXy0BnZAeZNh/l6QprEmRpZ6Yk332YL/+15VjFXi
i5wflJUKTjeIW64mgGKXL22lImmLb5Q6kNI1dYEQBnppEQLTi9DAH65LU75hC5iM5Mxn
xLlA==
X-Gm-Message-State: AOJu0YzNV8DuqnbbV4Y9pms3dkPWiKozuxPUZiBGn0UNjTSP38UXGhT8
OTm4jLhOXiPQP7hFO0DCtzyRo0lUDAx7xL53YuM/j4AaBl/EkxkfIRC/0Q==
X-Gm-Gg: ASbGncugzyQ5Pajwx2l853oZAS9kqB9VeGQEkkiowuvvMyy1jTIMXqIbYH2YfEYzHVl
EitJT8kZOiF1pV73CRX7XYVfHTkYq3JH5Up/mHUPt8RB2k2TzQWnm5U8lxW0CqFHfIAhzTQzjeR
zb/t9dxSeTqaDVBWgeh2GQt4rvvsCPfwyVovNA/kd+ZOps0Zg18RlJvBeXSEF0h5ddss1WP+y3/
exLgo/KSM9Tvz/Y+XSlScyrGC5qSmZbPOQ8kMbOAQq0b0BEE6hWcAPlHjf9N6Kgt4Gb2LqBeGdA
f6oiz2AhgIg+I8EPpeNJDuO4oE3UrF/xzXtcjqcR4lNZItkZxXCicv0Tjefh0tbdBpGAwhGMezc
rigjx
X-Google-Smtp-Source: AGHT+IGZnnl1xcYWKpTX+i/6zSIfw9ki/K7JAytsAB+nBv894wKIYgz8bSkoimf46txY7Y+ZGSdaWw==
X-Received: by 2002:a05:6000:22c3:b0:3a0:8096:7c7f with SMTP id
ffacd0b85a97d-3a080967c99mr4984148f8f.50.1745871860062;
Mon, 28 Apr 2025 13:24:20 -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-3a073e5e351sm11781283f8f.100.2025.04.28.13.24.19
(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
Mon, 28 Apr 2025 13:24:19 -0700 (PDT)
Received: by mariano (Postfix, from userid 1000)
id 68527BFCE8; Mon, 28 Apr 2025 22:24:18 +0200 (CEST)
Date: Mon, 28 Apr 2025 22:24:18 +0200
From: Stefano Sabatini <stefasab@gmail.com>
To: "softworkz ." <softworkz@hotmail.com>
Message-ID: <aA/j8udHPaUshX70@mariano>
Mail-Followup-To: "softworkz ." <softworkz-at-hotmail.com@ffmpeg.org>,
FFmpeg development discussions and patches <ffmpeg-devel@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>
<aA/dbPipiEq0ZPw1@mariano>
<DM8P223MB03654FE01A37E77E0968DEC7BA812@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM>
MIME-Version: 1.0
Content-Disposition: inline
In-Reply-To: <DM8P223MB03654FE01A37E77E0968DEC7BA812@DM8P223MB0365.NAMP223.PROD.OUTLOOK.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: 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/aA%2Fj8udHPaUshX70@mariano/>
List-Archive: <https://master.gitmailbox.com/ffmpegdev/>
List-Post: <mailto:ffmpegdev@gitmailbox.com>
On date Monday 2025-04-28 20:05:25 +0000, softworkz . wrote:
>
>
> > -----Original Message-----
> > From: Stefano Sabatini <stefasab@gmail.com>
> > Sent: Montag, 28. April 2025 21:56
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Cc: softworkz <softworkz@hotmail.com>
> > Subject: Re: [FFmpeg-devel] [PATCH v7 02/13] fftools/textformat: Apply quality
> > improvements
> >
> > 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?
>
> OK.
>
>
> >
> > > +
> > > 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
>
> The dynamic allocation does not happen until the buffer content size
> does not fit anymore into the stack-alloced memory.
Yes, but is this ever happening? Since in this case the size of the
buffer is guaranteed to be short (since it's the max return size of
the UTF8 decode function). It won't hurt, but it won't help either so
I'd rather keep as is and fix the indent weirdness (the , in place of
; for instance) and the ret.
> >
> > > + 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.
>
> Let's discuss this in the other thread (Shaping...).
> Did you see my recent reply?
Yes, ok.
_______________________________________________
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".