linux-ext4 - Re: [PATCH 03/10 v3] ext4: add physical block and status member into extent status tree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 29 Jan 2013 13:34:15 +0800
From:	Zheng Liu <gnehzuil.liu@...il.com>
To:	Theodore Ts'o <tytso@....edu>
Cc:	linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	Zheng Liu <wenqing.lz@...bao.com>
Subject: Re: [PATCH 03/10 v3] ext4: add physical block and status member into
 extent status tree

On Mon, Jan 28, 2013 at 10:03:53PM -0500, Theodore Ts'o wrote:
> On Wed, Jan 23, 2013 at 08:03:53PM +0800, Zheng Liu wrote:
> > +	ext4_fsblk_t es_pblk : 62;	/* first physical block */
> > +	ext4_fsblk_t es_status : 2;	/* record the status of extent */
> 
> I'll accept this for now but note that ext4_fsblk_t is typedefed to be
> an unsigned long long, and C99 only guarantees that bitfields can be
> made from Bool, signed int, and unsigned int.  Gcc accepts unsigned
> long long based bit fields as an extension, but it's not portable
> code.  This is kernel code, though, and we have plenty more gcc
> specific code....

Thanks for pointing out.  When I tried to implement this code, there are
two choices.  One is like this that bit field is used.  IMHO it is clear
enough, although it is not portable.

Another choice is like this:

        struct extent_status {
                ...
                ext4_fsblk_t es_pblk;   /* first physical block */
        };

        #define EXTENT_STATUS_WRITTEN   (1ULL << 60)
        #define EXTENT_STATUS_UNWRITTEN (1ULL << 61)
        #define EXTENT_STATUS_DELAYED   (1ULL << 62)

When we want to set extent status, we will need to do like the following:

        es->es_pblk |= EXTENT_STATUS_WRITTEN;

This can make us avoid non-protable code.  I am happy to refine this
patch if you think the latter one is better.

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