From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hitoshi Mitake Subject: Re: [PATCH] mkfs: check sizes of important structs at build time Date: Mon, 06 Jan 2014 00:22:22 +0900 Message-ID: <87wqiea1xt.wl%mitake.hitoshi@gmail.com> References: <1388842171-16105-1-git-send-email-mitake.hitoshi@gmail.com> <20140105.010843.356918311.konishi.ryusuke@lab.ntt.co.jp> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:message-id:from:to:cc:subject:in-reply-to:references :user-agent:mime-version:content-type; bh=1uBaX4V/aeS5cgLy2PdysBTMrO+esl/ZeSQwmDQ5o5A=; b=Afmih4y7qauvkHkJIPY34pZ6meTbINldkzPluyNYAtupj4VFke3btd6yZxy4pjR273 ETIXl6SkYTTyIAu+UOiAsJmTZOWG4b+ksh44Q4O4Rcerm6hTt52uTncVD/jVuy9WdUBd D9Dq5WbGU/1Phs3sZuHDrWA+gCJZD2n2uojO78f6q5a1V2d/b3zpx3j1oOpw3QoQ9n1V qJscSNSyD+dTCfvbXiprU2eOIm/kmRWkhxTLkHFf7Sgn5N4rjydreED3BD1eSLjZhMrI Xu5P4JK9iDLGehMoa4kBi6JoKrv5Zpw9ykAcpCWYywYL6/HhwrvaHK+kxytWonl+YCLo LDKg== In-Reply-To: <20140105.010843.356918311.konishi.ryusuke-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org> Sender: linux-nilfs-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-ID: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Ryusuke Konishi Cc: Hitoshi Mitake , linux-nilfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Hitoshi Mitake , Vyacheslav Dubeyko At Sun, 05 Jan 2014 01:08:43 +0900 (JST), Ryusuke Konishi wrote: > > On Sat, 4 Jan 2014 22:29:31 +0900, 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); > > ... > > > > build process of mkfs.o causes errors like this: > > > > gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I../.. -I../../include -Wall -g -O2 -MT mkfs.o -MD -MP -MF .deps/mkfs.Tpo -c -o mkfs.o mkfs.c > > mkfs.c: In function 'nilfs_check_ondisk_sizes': > > mkfs.c:429:2: error: negative width in bit-field '' > > mkfs.c:430:2: error: negative width in bit-field '' > > mkfs.c:431:2: error: negative width in bit-field '' > > mkfs.c:432:2: error: negative width in bit-field '' > > mkfs.c:433:2: error: negative width in bit-field '' > > mkfs.c:434:2: error: negative width in bit-field '' > > mkfs.c:435:2: error: negative width in bit-field '' > > > > Signed-off-by: Hitoshi Mitake > > This is an interesting patch. > > I am inclined to apply this since the every test in the > nilfs_check_ondisk_sizes function is static. > > If we will add a new check that depends on block size in a future, we > need to add a separate runtime check function as Vyacheslav wrote, but > I think you are doing right thing. > > One my question is why you used bit operator. The BUILD_BUG_ON marcro > of kernel is implemented with negative array index. > Is there any reason for this ? If I remember correctly, I found in the BUILD_BUG_ON() in the code of Xen. I don't have any opinion about how we implement the check. If you like the way of array with negative length, I will employ it in v2. BTW, there is another approach of the implementation. #define static_assert(a, b) do { switch (0) case 0: case (a): ; } while (0) # from xv6: http://pdos.csail.mit.edu/6.828/2012/xv6.html This duplicated case of switch statement can be used to implement BUILD_BUG_ON() (the b can be used as an error message). Which one do you like? 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