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). > > 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. Cheers, Andreas -- Andreas Dilger Sr. Staff Engineer, Lustre Group Sun Microsystems of Canada, 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
