From: Tyler Retzlaff <roretzla@linux.microsoft.com>
To: "Morten Brørup" <mb@smartsharesystems.com>
Cc: Ferruh Yigit <ferruh.yigit@amd.com>,
dev@dpdk.org, rmody@marvell.com, timothy.mcdaniel@intel.com,
matan@nvidia.com, viacheslavo@nvidia.com, ruifeng.wang@arm.com,
zhoumin@loongson.cn, drc@linux.vnet.ibm.com, kda@semihalf.com,
bruce.richardson@intel.com, konstantin.v.ananyev@yandex.ru
Subject: Re: [PATCH v7 4/4] eal: add nonnull and access function attributes
Date: Tue, 17 Jan 2023 13:16:56 -0800 [thread overview]
Message-ID: <20230117211656.GA30892@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> (raw)
In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35D8767B@smartserver.smartshare.dk>
On Tue, Jan 17, 2023 at 09:19:22AM +0100, Morten Brørup wrote:
> > From: Ferruh Yigit [mailto:ferruh.yigit@amd.com]
> > Sent: Monday, 16 January 2023 18.02
> >
> > On 1/16/2023 1:07 PM, Morten Brørup wrote:
> > > Add nonnull function attribute to help the compiler detect a NULL
> > > pointer being passed to a function not accepting NULL pointers as an
> > > argument at build time.
> > >
> > > Add access function attributes to tell the compiler how a function
> > > accesses memory pointed to by its pointer arguments.
> > >
> > > Add these attributes to the rte_memcpy() function, as the first in
> > > hopefully many to come.
> > >
> > > Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> > > Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > > ---
>
> [...]
>
> > > +/**
> > > + * Tells compiler that the pointer arguments must be non-null.
> > > + *
> > > + * @param ...
> > > + * Comma separated list of parameter indexes of pointer
> > arguments.
> > > + */
> > > +#if defined(RTE_CC_GCC) || defined(RTE_CC_CLANG)
> > > +#define __rte_nonnull_params(...) \
> > > + __attribute__((nonnull(__VA_ARGS__)))
> > > +#else
> > > +#define __rte_nonnull_params(...)
> > > +#endif
> > > +
> >
> > What do you think to have a namespace for macros like
> > '__rte_param_xxx',
> > so name macros as:
> > __rte_param_nonull
> > __rte_param_read_only
> > __rte_param_write_only
> >
> > No strong opinion, it just feels tidier this way
>
> Being a proponent of the world_country_city naming scheme myself, I would usually agree with this proposal.
>
> However, in the future, we might add macros without _param for use along with the function parameters, e.g.:
>
> int foo(int bar __rte_nonnull __rte_read_only);
>
> So I decided for this order in the names (treating nonnull/access_mode as "country" and param/params as "city"), also somewhat looking at the __rte_deprecated and __rte_deprecated_msg(msg) macros.
>
> I have no strong preference either, so if anyone does, please speak up.
>
> Slightly related, also note this:
>
> The nonnull macro is plural (_params), because it can take multiple pointer parameter indexes.
> The access mode macros are singular (_param), because they only take one pointer parameter index, and the optional size parameter index.
>
> I considered splitting up the access mode macros even more, making two variants of each, e.g. __rte_read_only_param(ptr_index) and __rte_read_only_param_size(ptr_index, size_index), but concluded that it would be excruciatingly verbose. The only purpose would be to reduce the risk of using them incorrectly. I decided against it, thinking that any developer clever enough to use these macros is also clever enough to understand how to use them (or at least read their parameter descriptions to learn how).
>
microsoft also has a tool & annotation vehicle for this type of stuff.
this discussion has caused me to wonder what happens if we would like to
add additional annotations for other tools. just load on the annotations
and expand them empty conditionally?
https://learn.microsoft.com/en-us/cpp/code-quality/using-sal-annotations-to-reduce-c-cpp-code-defects?view=msvc-170
anyway, just a thought. no serious response required here.
next prev parent reply other threads:[~2023-01-17 21:16 UTC|newest]
Thread overview: 77+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-02 15:34 [PATCH] " Morten Brørup
2022-12-02 20:02 ` Tyler Retzlaff
2022-12-03 14:22 ` [PATCH v2] " Morten Brørup
2022-12-05 10:17 ` Ruifeng Wang
2022-12-12 7:40 ` Morten Brørup
2022-12-28 10:27 ` [PATCH v3 1/2] " Morten Brørup
2022-12-28 10:27 ` [PATCH v3 2/2] net/bnx2x: fix warnings about rte_memcopy lengths Morten Brørup
2022-12-28 11:40 ` [PATCH v4 1/2] eal: add nonnull and access function attributes Morten Brørup
2022-12-28 11:40 ` [PATCH v4 2/2] net/bnx2x: fix warnings about rte_memcpy lengths Morten Brørup
2022-12-28 15:10 ` [PATCH v5 1/4] eal: add nonnull and access function attributes Morten Brørup
2022-12-28 15:10 ` [PATCH v5 2/4] net/bnx2x: fix warnings about rte_memcpy lengths Morten Brørup
2022-12-28 16:13 ` Stanisław Kardach
2022-12-28 16:38 ` Morten Brørup
2022-12-28 17:03 ` Stephen Hemminger
2022-12-28 17:37 ` Morten Brørup
2023-01-09 10:36 ` David Marchand
2022-12-28 15:10 ` [PATCH v5 3/4] event/dlb2: remove superfluous rte_memcpy Morten Brørup
2022-12-28 17:01 ` Stephen Hemminger
2022-12-28 15:10 ` [PATCH v5 4/4] net/mlx5: fix warning about rte_memcpy length Morten Brørup
2023-01-09 11:08 ` [PATCH v5 1/4] eal: add nonnull and access function attributes Thomas Monjalon
2023-01-09 12:16 ` Morten Brørup
2023-01-09 11:22 ` David Marchand
2023-01-09 12:28 ` Morten Brørup
2023-01-16 12:49 ` Morten Brørup
2023-01-16 12:44 ` [PATCH v6 1/4] net/bnx2x: fix warnings about rte_memcpy lengths Morten Brørup
2023-01-16 12:44 ` [PATCH v6 2/4] event/dlb2: remove superfluous rte_memcpy Morten Brørup
2023-01-16 12:44 ` [PATCH v6 3/4] net/mlx5: fix warning about rte_memcpy length Morten Brørup
2023-01-16 12:44 ` [PATCH v6 4/4] eal: add nonnull and access function attributes Morten Brørup
2023-01-16 13:07 ` [PATCH v7 1/4] net/bnx2x: fix warnings about rte_memcpy lengths Morten Brørup
2023-01-16 13:07 ` [PATCH v7 2/4] event/dlb2: remove superfluous rte_memcpy Morten Brørup
2023-02-09 16:51 ` Morten Brørup
2023-02-09 18:50 ` Sevincer, Abdullah
2023-02-10 7:43 ` Morten Brørup
2024-02-23 13:19 ` Jerin Jacob
2024-02-23 13:49 ` [PATCH v8] net/bnx2x: fix warnings about rte_memcpy lengths Morten Brørup
2024-02-23 14:00 ` [PATCH v9] " Morten Brørup
2024-02-26 8:34 ` Jerin Jacob
2024-02-26 14:47 ` Morten Brørup
2024-02-27 11:00 ` Jerin Jacob
2024-02-27 11:27 ` Morten Brørup
2024-02-27 19:06 ` Stephen Hemminger
2024-02-28 9:02 ` Bruce Richardson
2024-02-28 8:27 ` Raslan Darawsheh
2024-02-26 21:45 ` Stephen Hemminger
2023-01-16 13:07 ` [PATCH v7 3/4] net/mlx5: fix warning about rte_memcpy length Morten Brørup
2023-02-09 16:54 ` Morten Brørup
2023-02-10 9:13 ` Slava Ovsiienko
2024-03-13 10:00 ` Raslan Darawsheh
2023-01-16 13:07 ` [PATCH v7 4/4] eal: add nonnull and access function attributes Morten Brørup
2023-01-16 17:02 ` Ferruh Yigit
2023-01-17 8:19 ` Morten Brørup
2023-01-17 21:16 ` Tyler Retzlaff [this message]
2023-01-18 8:31 ` Morten Brørup
2023-01-18 17:23 ` Stephen Hemminger
2023-01-31 11:14 ` David Marchand
2023-01-31 12:23 ` Morten Brørup
2023-01-31 18:26 ` Tyler Retzlaff
2023-01-31 22:52 ` Thomas Monjalon
2023-02-01 12:50 ` Morten Brørup
2023-02-01 13:15 ` Thomas Monjalon
2023-02-06 16:11 ` David Marchand
2023-02-06 16:49 ` Morten Brørup
2023-02-06 17:28 ` Tyler Retzlaff
2023-05-08 12:32 ` Morten Brørup
2023-10-13 23:31 ` Morten Brørup
2023-04-04 13:41 ` Morten Brørup
2023-04-04 13:51 ` David Marchand
2023-04-04 14:01 ` Morten Brørup
2023-04-04 14:19 ` David Marchand
2023-02-09 16:49 ` [PATCH v7 1/4] net/bnx2x: fix warnings about rte_memcpy lengths Morten Brørup
2023-02-26 9:21 ` Marvell QLogic bnx2x PMD support status Morten Brørup
2023-03-09 10:25 ` [PATCH v7 1/4] net/bnx2x: fix warnings about rte_memcpy lengths David Marchand
2023-03-09 10:33 ` Thomas Monjalon
2023-03-09 16:11 ` [EXT] " Akhil Goyal
[not found] ` <SJ0PR18MB390039A8C34E485F8D2D518EA1B59@SJ0PR18MB3900.namprd18.prod.outlook.com>
2023-03-09 16:19 ` Devendra Singh Rawat
2023-03-09 16:23 ` Stephen Hemminger
2024-02-23 12:39 ` Jerin Jacob
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=20230117211656.GA30892@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net \
--to=roretzla@linux.microsoft.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=drc@linux.vnet.ibm.com \
--cc=ferruh.yigit@amd.com \
--cc=kda@semihalf.com \
--cc=konstantin.v.ananyev@yandex.ru \
--cc=matan@nvidia.com \
--cc=mb@smartsharesystems.com \
--cc=rmody@marvell.com \
--cc=ruifeng.wang@arm.com \
--cc=timothy.mcdaniel@intel.com \
--cc=viacheslavo@nvidia.com \
--cc=zhoumin@loongson.cn \
/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).