Re: [PATCH 3/5] reiserfs: rework reiserfs_warning
Jeff Mahoney wrote: > Hans Reiser wrote: > > >Jeff, thanks so kindly for cleaning all this up, it must have been very > >tedious, so extra thanks for it. > > >I will now quibble about some trivia > > >Hans > > >Jeff Mahoney wrote: > > >>ReiserFS warnings can be somewhat inconsistent. > >>In some cases: > >>* a unique identifier may be associated with it > >>* the function name may be included > >>* the device may be printed separately > >> > >>This patch aims to make warnings more consistent. reiserfs_warning() > prints > >>the device name, so printing it a second time is not required. The > function > >>name for a warning is always helpful in debugging, so it is now > automatically > >>inserted into the output. Hans has stated that every warning should have > >>a unique identifier. Some cases lack them, others really shouldn't > have them. > >> > >> > >What cases should not have them? > > > I don't think that "routine" messages should have identifiers associated > with them. I guess in a more exact sense, messages that are directly > associated with user input, like mount option parsing, finding the > superblock, an unfinished reiserfsck, or enabling CONFIG_REISERFS_CHECK. I disagree, please don't remove identifiers. > > I guess a quick visual search for NO_ID in the patch would be the best > way of expressing this. I could be convinced otherwise, and that's why I > made two separate #defines for a missing id or deliberately no id. > > -Jeff > > -- > Jeff Mahoney > SuSE Labs
Re: [PATCH 3/5] reiserfs: rework reiserfs_warning
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hans Reiser wrote: > Jeff, thanks so kindly for cleaning all this up, it must have been very > tedious, so extra thanks for it. > > I will now quibble about some trivia > > Hans > > Jeff Mahoney wrote: > >>ReiserFS warnings can be somewhat inconsistent. >>In some cases: >>* a unique identifier may be associated with it >>* the function name may be included >>* the device may be printed separately >> >>This patch aims to make warnings more consistent. reiserfs_warning() prints >>the device name, so printing it a second time is not required. The function >>name for a warning is always helpful in debugging, so it is now automatically >>inserted into the output. Hans has stated that every warning should have >>a unique identifier. Some cases lack them, others really shouldn't have them. >> >> > What cases should not have them? I don't think that "routine" messages should have identifiers associated with them. I guess in a more exact sense, messages that are directly associated with user input, like mount option parsing, finding the superblock, an unfinished reiserfsck, or enabling CONFIG_REISERFS_CHECK. I guess a quick visual search for NO_ID in the patch would be the best way of expressing this. I could be convinced otherwise, and that's why I made two separate #defines for a missing id or deliberately no id. - -Jeff - -- Jeff Mahoney SuSE Labs -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.0 (GNU/Linux) iD8DBQFC3E2uLPWxlyuTD7IRAufMAJ9RB1jrQalthIExa/4h+IouWrjr7gCcC34j wGWoF4EI5kcfAWaL4UScBWo= =mX/a -END PGP SIGNATURE-
Re: [PATCH 3/5] reiserfs: rework reiserfs_warning
Jeff, thanks so kindly for cleaning all this up, it must have been very tedious, so extra thanks for it. I will now quibble about some trivia Hans Jeff Mahoney wrote: > ReiserFS warnings can be somewhat inconsistent. > In some cases: > * a unique identifier may be associated with it > * the function name may be included > * the device may be printed separately > > This patch aims to make warnings more consistent. reiserfs_warning() prints > the device name, so printing it a second time is not required. The function > name for a warning is always helpful in debugging, so it is now automatically > inserted into the output. Hans has stated that every warning should have > a unique identifier. Some cases lack them, others really shouldn't have them. > > What cases should not have them? > reiserfs_warning() now expects an id associated with each message. In the > event that it is missing, MISSING_ID is used. In the case where one is simply > not desired, NO_ID is used. Both of these are currently #define'd to NULL, > but may be changed in the future. > >