From: Bruce Richardson <bruce.richardson@intel.com>
To: Tyler Retzlaff <roretzla@linux.microsoft.com>
Cc: <dev@dpdk.org>, <ciara.power@intel.com>,
<david.marchand@redhat.com>, <thomas@monjalon.net>
Subject: Re: [PATCH v3] telemetry: use portable syntax to initialize array
Date: Tue, 4 Apr 2023 17:08:47 +0100 [thread overview]
Message-ID: <ZCxLj7HAz4xSA21C@bricha3-MOBL.ger.corp.intel.com> (raw)
In-Reply-To: <20230404155430.GD23247@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
On Tue, Apr 04, 2023 at 08:54:30AM -0700, Tyler Retzlaff wrote:
> On Tue, Apr 04, 2023 at 09:51:04AM +0100, Bruce Richardson wrote:
> > On Mon, Apr 03, 2023 at 11:59:25AM -0700, Tyler Retzlaff wrote:
> > > Use of ranges in designated initialization are a non-standard gcc
> > > extension. Use loops to initialize permitted characters on first use.
> > >
> > > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> >
> > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> >
> > > ---
> > > lib/telemetry/telemetry_data.c | 22 +++++++++++++++-------
> > > 1 file changed, 15 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/lib/telemetry/telemetry_data.c b/lib/telemetry/telemetry_data.c
> > > index 2bac2de..562b387 100644
> > > --- a/lib/telemetry/telemetry_data.c
> > > +++ b/lib/telemetry/telemetry_data.c
> > > @@ -152,13 +152,21 @@
> > > static bool
> > > valid_name(const char *name)
> > > {
> > > - char allowed[128] = {
> > > - ['0' ... '9'] = 1,
> > > - ['A' ... 'Z'] = 1,
> > > - ['a' ... 'z'] = 1,
> > > - ['_'] = 1,
> > > - ['/'] = 1,
> > > - };
> > > + int index;
> >
> > My preference would be to limit the scope of index to the if block, but ok
> > to keep as here.
>
> yes, mine too. but i forgot we aren't C99 (yet) so i had to move it for
> this patch.
>
We do allow variable declarations within blocks, so no need to move it up
here. However, I believe our coding standards currently require them at the
*top* of each block, not in the middle - but also not just at the top of
each function. See [1] for the details. [For some reason this is in the
sub-section under "indentation" in the guide!]
[1] https://doc.dpdk.org/guides-22.11/contributing/coding_style.html#local-variables
> >
> > [In fact, when we switch to C11, I'd love to see the coding standards
> > relaxed to allow loop variable definition inside the for statement itself]
>
> i'm for it, in fact for any variable i prefer declaration at or near
> first use because it let's me const more. but i know that can be an
> unpopular opinion so no need to hit 'r' to tell me off, i will follow
> any and all documented convention the community has.
>
For const, our coding standards already call this out as an exception,
where it's allowed to define a variable in the middle of a block.
Generally though, I too prefer vars to be defined at site of first use,
rather than all together at the top of the block. Makes it easier when
commenting out code for debugging, not to get unused var warnings.
/Bruce
next prev parent reply other threads:[~2023-04-04 16:13 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-03 16:30 [PATCH 0/2] improve code portability Tyler Retzlaff
2023-04-03 16:30 ` [PATCH 1/2] telemetry: use malloc instead of variable length array Tyler Retzlaff
2023-04-03 17:17 ` Tyler Retzlaff
2023-04-03 20:19 ` Stephen Hemminger
2023-04-03 20:40 ` Tyler Retzlaff
2023-04-04 8:47 ` Bruce Richardson
2023-04-04 16:24 ` Tyler Retzlaff
2023-04-04 16:28 ` Bruce Richardson
2023-04-04 16:44 ` Tyler Retzlaff
2023-04-04 17:25 ` Bruce Richardson
2023-04-04 17:34 ` Tyler Retzlaff
2023-04-05 1:20 ` Stephen Hemminger
2023-04-05 8:53 ` Bruce Richardson
2023-04-05 1:04 ` Stephen Hemminger
2023-04-05 8:54 ` Bruce Richardson
2023-04-05 15:25 ` Tyler Retzlaff
2023-04-05 15:30 ` Dmitry Kozlyuk
2023-04-05 15:37 ` Stephen Hemminger
2023-04-05 15:47 ` Bruce Richardson
2023-04-03 16:30 ` [PATCH 2/2] telemetry: use portable syntax to initialize array Tyler Retzlaff
2023-04-03 17:04 ` [PATCH 0/2] improve code portability Bruce Richardson
2023-04-03 17:35 ` Tyler Retzlaff
2023-04-03 18:47 ` [PATCH v2] " Tyler Retzlaff
2023-04-03 18:47 ` [PATCH v2] telemetry: use portable syntax to initialize array Tyler Retzlaff
2023-04-03 18:59 ` [PATCH v3] improve code portability Tyler Retzlaff
2023-04-03 18:59 ` [PATCH v3] telemetry: use portable syntax to initialize array Tyler Retzlaff
2023-04-04 8:51 ` Bruce Richardson
2023-04-04 15:54 ` Tyler Retzlaff
2023-04-04 16:08 ` Bruce Richardson [this message]
2023-04-04 9:01 ` Konstantin Ananyev
2023-04-04 15:59 ` Tyler Retzlaff
2023-04-04 16:19 ` Bruce Richardson
2023-04-04 16:28 ` Tyler Retzlaff
2023-04-04 18:09 ` [PATCH v4] improve code portability Tyler Retzlaff
2023-04-04 18:09 ` [PATCH v4] telemetry: remove non-portable array initialization syntax Tyler Retzlaff
2023-04-05 8:56 ` Bruce Richardson
2023-04-05 15:27 ` Tyler Retzlaff
2023-04-05 18:52 ` [PATCH v5] improve code portability Tyler Retzlaff
2023-04-05 18:52 ` [PATCH v5] telemetry: remove non-portable array initialization syntax Tyler Retzlaff
2023-05-24 20:54 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZCxLj7HAz4xSA21C@bricha3-MOBL.ger.corp.intel.com \
--to=bruce.richardson@intel.com \
--cc=ciara.power@intel.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=roretzla@linux.microsoft.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).