Linux NILFS development
 help / color / mirror / Atom feed
From: Hitoshi Mitake <mitake.hitoshi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Vyacheslav Dubeyko <slava-yeENwD64cLxBDgjK7y7TUQ@public.gmane.org>
Cc: Hitoshi Mitake
	<mitake.hitoshi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	linux-nilfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Hitoshi Mitake
	<mitake.hitoshi-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org>
Subject: Re: [PATCH] mkfs: check sizes of important structs at build time
Date: Mon, 06 Jan 2014 00:17:13 +0900	[thread overview]
Message-ID: <87y52ua26e.wl%mitake.hitoshi@gmail.com> (raw)
In-Reply-To: <2276BC9A-0688-4566-8FA8-D280E6F71F5F-yeENwD64cLxBDgjK7y7TUQ@public.gmane.org>

At Sat, 4 Jan 2014 18:39:58 +0300,
Vyacheslav Dubeyko wrote:
> 
> 
> On Jan 4, 2014, at 4:54 PM, Hitoshi Mitake wrote:
> 
> > On Sat, Jan 4, 2014 at 11:52 PM, Vyacheslav Dubeyko <slava-yeENwD64cLxBDgjK7y7TUQ@public.gmane.org> wrote:
> >> 
> >> On Jan 4, 2014, at 4:29 PM, Hitoshi Mitake wrote:
> >> 
> >>> Current nilfs_check_ondisk_sizes() checks sizes of important structs
> >>> at run time. The checking should be done at build time. This patch
> >>> adds a new macro, BUILD_BUG_ON(), for this purpose. It is similar to
> >>> static_assert() of C++11. If an argument is true, the macro causes a
> >>> bulid error.
> >>> 
> >>> Below is an example of BUILD_BUG_ON(). When the checked conditions are
> >>> true like below:
> >>> 
> >>> /* intentional change for testing BUILD_BUG_ON() */
> >>> 
> >>> static __attribute__((used)) void nilfs_check_ondisk_sizes(void)
> >>> {
> >>>      BUILD_BUG_ON(sizeof(struct nilfs_inode) > NILFS_MIN_BLOCKSIZE);
> >> 
> >> So, why do we need to have function for the case of checking on compilation
> >> phase?
> > 
> > Just for excluding the checking from other part of code and improve readability.
> > 
> 
> I think that we can have only macro instead of the function nilfs_check_ondisk_sizes().
> And this macros can be placed in the begin of main() call. I think that it will be enough
> for the compilation phase check.

Ah, I see.

> 
> >> 
> >> I suppose that we need to have some run-time check anyway. Your approach
> >> is correct for the current state of the code. But I feel a necessity in run-time check
> >> anyway. Maybe it looks like a paranoia. :) Maybe it needs to extend checking
> >> in this place.
> > 
> > Do you mean both of the build time check and the run time check? If
> > so, I agree with your opinion. I'll send v2 based on this policy.
> > 
> 
> I mean that block size can be different during volume creation and maybe
> it makes sense to extend a block size related checking for run-time phase.
> That's all. But right now I haven't any concrete suggestions.

Currently, the check is comparison between sizes of important structs and the
minimal block size. So we don't need a function for it. I think adding the
function for runtime checking when nilfs requires it would be enough.

Thanks,
Hitoshi

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

  parent reply	other threads:[~2014-01-05 15:17 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-04 13:29 [PATCH] mkfs: check sizes of important structs at build time Hitoshi Mitake
     [not found] ` <1388842171-16105-1-git-send-email-mitake.hitoshi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-01-04 14:52   ` Vyacheslav Dubeyko
     [not found]     ` <EF28F22A-C43F-41C9-A5B9-8597C5879E3E-yeENwD64cLxBDgjK7y7TUQ@public.gmane.org>
2014-01-04 13:54       ` Hitoshi Mitake
     [not found]         ` <CAE1WaKLr1EuovgHgXQa1o9LQVk1fRkUXbDrGiKfsdTB347ieqg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-01-04 15:39           ` Vyacheslav Dubeyko
     [not found]             ` <2276BC9A-0688-4566-8FA8-D280E6F71F5F-yeENwD64cLxBDgjK7y7TUQ@public.gmane.org>
2014-01-05 15:17               ` Hitoshi Mitake [this message]
2014-01-04 16:08   ` Ryusuke Konishi
     [not found]     ` <20140105.010843.356918311.konishi.ryusuke-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org>
2014-01-05 15:22       ` Hitoshi Mitake
     [not found]         ` <87wqiea1xt.wl%mitake.hitoshi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-01-05 17:17           ` Ryusuke Konishi

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=87y52ua26e.wl%mitake.hitoshi@gmail.com \
    --to=mitake.hitoshi-re5jqeeqqe8avxtiumwx3w@public.gmane.org \
    --cc=linux-nilfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mitake.hitoshi-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org \
    --cc=slava-yeENwD64cLxBDgjK7y7TUQ@public.gmane.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