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@xxxxxxxxxx> Google, Inc. -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
