[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1226945475.23804.17.camel@bobble.smo.corp.google.com>
Date: Mon, 17 Nov 2008 10:11:15 -0800
From: Frank Mayhar <fmayhar@...gle.com>
To: Andreas Dilger <adilger@....com>
Cc: linux-ext4@...r.kernel.org, Michael Rubin <mrubin@...gle.com>
Subject: Re: [RFC PATCH 1/1] Allow ext4 to run without a journal, take 2.
On Mon, 2008-11-17 at 12:08 -0600, Andreas Dilger wrote:
> On Nov 17, 2008 09:20 -0800, Frank Mayhar wrote:
> > > > + if (handle->h_transaction == NULL)
> > > > + return 0;
> > >
> > > Does this ever happen? Ah, is this the case where the handle is pointing
> > > to the ext4_sb_info()? I think I'd prefer to have a magic value here,
> > > so that it isn't possible to accidentally dereference a pointer that
> > > happens to have NULL data in it.
> >
> > So some magic value that gets stuffed into the pointer? Or just a magic
> > pointer value that gets stuffed into 'handle'?
>
> I mean a magic value that is stuffed into the s_nojournal_flag pointer
> instead of just using NULL (which is a very common value of pointers to
> random memory). Ideally this value will be chosen in a way that it is
> unlikely to be a valid pointer (e.g. 0x01234567 for 32-bit machines,
> 0x0123456789abcdef for 64-bit machines).
Okay, makes sense, will do.
> > > So, this appears to be the place where ext4_sb_info->s_nojournal_flag is
> > > actually used. To make this more clear, it would be better to do actually
> > > reference this variable here so that it is easier for the reader to follow.
> > >
> > > current->journal_info = &EXT4_SB(sb)->s_nojournal_flag;
> >
> > Hmm. Okay, I'll have to think about this a bit; I haven't had my coffee
> > yet.
>
> This is functionally equivalent to the method you are using (i.e. it stores
> the pointer to the start of the struct), except that it allows the reader
> to find where this field is used.
Yeah, that's what I thought, but like I said, I hadn't had my coffee
yet. :-)
--
Frank Mayhar <fmayhar@...gle.com>
Google, Inc.
--
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
Powered by blists - more mailing lists