linux-sparse.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christopher Li <sparse@chrisli.org>
To: Josh Triplett <josh@joshtriplett.org>
Cc: linux-sparse@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] New attribute designated_init: mark a struct as requiring designated init
Date: Sat, 10 Oct 2009 03:33:20 -0700	[thread overview]
Message-ID: <70318cbf0910100333m128e3500vb1659a55a9f49768@mail.gmail.com> (raw)
In-Reply-To: <20091010085732.GA10923@feather>

On Sat, Oct 10, 2009 at 1:58 AM, Josh Triplett <josh@joshtriplett.org> wrote:
> Some structure types provide a set of fields of which most users will
> only initialize the subset they care about.  Users of these types should
> always use designated initializers, to avoid relying on the specific
> structure layout.  Examples of this type of structure include the many

The patch is very well written with nice documentations and test case.
It applies and runs fine.

I am curious weather this is some thing the kernel developers want to
use. Please speak up if you want to annotate the kernel structure to
issue such warning. If some one use it, I have no problem adding it to
sparse.

I am not sure how useful this is yet.  If the structure is changed, most
likely the positional initialization will fail due to type mismatching.
Some real life example how this feature can expose some otherwise
hard to detect bug would be nice.

With this approach, we need to annotate the kernel to benefit from it.
Another idea is that we can find out how different part of the kernel
initialize the same structure. If most of them using designated init then
the few non-conforming can get a warning. This approach is more
complicate.  But it does not need to change the kernel.


> +                               warning(e->pos, "%s%s%spositional init of field in %s %s, declared with attribute designated_init",
> +                                       ctype->ident ? "in initializer for " : "",
> +                                       ctype->ident ? ctype->ident->name : "",

ident->name has no guarantee of terminating by NUL.
You want to use "%.*s" with ident->size, ident->name here.

Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2009-10-10 10:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-10  8:58 [PATCH] New attribute designated_init: mark a struct as requiring designated init Josh Triplett
2009-10-10 10:33 ` Christopher Li [this message]
2009-10-10 16:03   ` Josh Triplett
2009-10-12  5:18     ` Christopher Li
2009-10-12  6:42       ` Josh Triplett

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=70318cbf0910100333m128e3500vb1659a55a9f49768@mail.gmail.com \
    --to=sparse@chrisli.org \
    --cc=josh@joshtriplett.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sparse@vger.kernel.org \
    /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).