Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c
Paul Mundt wrote: > On Fri, Jan 11, 2008 at 12:04:14PM +0100, Roel Kluin wrote: >> Paul Mundt wrote: >>> Take a look at how CONFIG_PCMCIA_DEBUG is handled. >> In drivers/pcmcia/Makefile, when CONFIG_PCMCIA_DEBUG=y, it gives >> EXTRA_CFLAGS += -DDEBUG >> which causes the definition of DEBUG as a macro, with definition 1. >> >>> With DEBUG()->pr_debug() conversion here you've silently dropped the >>> __func__ prefixing. Note that dev_dbg() is usually preferred when you can >>> get a hold of a struct device pointer, as it takes care of prettifying >>> the output with the driver name and so on, rather than the convention of >>> adding a prefix. If you can't get at the struct device pointer, you'll >>> probably just want to insert the __func__ prefixing manually at the >>> callsites. > You're still changing the semantics here. The DEBUG() does __FUNCTION__ > prefixing, while pr_debug() does not. This should be something like > pr_debug("%s: ", __func__, ...); instead, if you want to maintain > consistency. Beyond that, this looks fine, yes. Somehow I overlooked your last point. Well, the original code had no semantics - it wasn't working - but I get your point. Below a patch to print the __func__'s. Note that all pr_debugs are in the same function. Maybe the function name should be printed in the first pr_debug only? If desired, I'll send another patch. -- Replace printk wrapper - with a syntax error - by pr_debug; keep printing the __func__'s. (DEBUG is defined 1 when CONFIG_PCMCIA_DEBUG is set) Signed-off-by: Roel Kluin <[EMAIL PROTECTED]> --- diff --git a/drivers/pcmcia/au1000_xxs1500.c b/drivers/pcmcia/au1000_xxs1500.c index ce9d5c4..b4bad6e 100644 --- a/drivers/pcmcia/au1000_xxs1500.c +++ b/drivers/pcmcia/au1000_xxs1500.c @@ -55,12 +55,6 @@ #define PCMCIA_NUM_SOCKS (PCMCIA_MAX_SOCK + 1) #define PCMCIA_IRQ AU1000_GPIO_4 -#if 0 -#define DEBUG(x,args...) printk(__FUNCTION__ ": " x,##args) -#else -#define DEBUG(x,args...) -#endif - static int xxs1500_pcmcia_init(struct pcmcia_init *init) { return PCMCIA_NUM_SOCKS; @@ -143,13 +137,13 @@ xxs1500_pcmcia_configure_socket(const struct pcmcia_configure *configure) if(configure->sock > PCMCIA_MAX_SOCK) return -1; - DEBUG("Vcc %dV Vpp %dV, reset %d\n", + pr_debug("%s: Vcc %dV Vpp %dV, reset %d\n", __func__, configure->vcc, configure->vpp, configure->reset); switch(configure->vcc){ case 33: /* Vcc 3.3V */ /* turn on power */ - DEBUG("turn on power\n"); + pr_debug("%s: turn on power\n", __func__); au_writel((au_readl(GPIO2_PINSTATE) & ~(1<<14))|(1<<30), GPIO2_OUTPUT); au_sync_delay(100); @@ -166,7 +160,7 @@ xxs1500_pcmcia_configure_socket(const struct pcmcia_configure *configure) } if (!configure->reset) { - DEBUG("deassert reset\n"); + pr_debug("%s: deassert reset\n", __func__); au_writel((au_readl(GPIO2_PINSTATE) & ~(1<<4))|(1<<20), GPIO2_OUTPUT); au_sync_delay(100); @@ -174,7 +168,7 @@ xxs1500_pcmcia_configure_socket(const struct pcmcia_configure *configure) GPIO2_OUTPUT); } else { - DEBUG("assert reset\n"); + pr_debug("%s: assert reset\n", __func__); au_writel(au_readl(GPIO2_PINSTATE) | (1<<4)|(1<<20), GPIO2_OUTPUT); } - To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c
On Fri, Jan 11, 2008 at 12:04:14PM +0100, Roel Kluin wrote: > Paul Mundt wrote: > > Take a look at how CONFIG_PCMCIA_DEBUG is handled. > > In drivers/pcmcia/Makefile, when CONFIG_PCMCIA_DEBUG=y, it gives > EXTRA_CFLAGS += -DDEBUG > which causes the definition of DEBUG as a macro, with definition 1. > > > With DEBUG()->pr_debug() conversion here you've silently dropped the > > __func__ prefixing. Note that dev_dbg() is usually preferred when you can > > get a hold of a struct device pointer, as it takes care of prettifying > > the output with the driver name and so on, rather than the convention of > > adding a prefix. If you can't get at the struct device pointer, you'll > > probably just want to insert the __func__ prefixing manually at the > > callsites. > > Ah, ok, then this should be right: > -- > Replace printk wrapper - with a syntax error - by pr_debug. > DEBUG is defined 1 when CONFIG_PCMCIA_DEBUG is set. > > Signed-off-by: Roel Kluin <[EMAIL PROTECTED]> > --- > diff --git a/drivers/pcmcia/au1000_xxs1500.c b/drivers/pcmcia/au1000_xxs1500.c > index ce9d5c4..8e6426b 100644 > --- a/drivers/pcmcia/au1000_xxs1500.c > +++ b/drivers/pcmcia/au1000_xxs1500.c > @@ -55,12 +55,6 @@ > #define PCMCIA_NUM_SOCKS (PCMCIA_MAX_SOCK + 1) > #define PCMCIA_IRQ AU1000_GPIO_4 > > -#if 0 > -#define DEBUG(x,args...) printk(__FUNCTION__ ": " x,##args) > -#else > -#define DEBUG(x,args...) > -#endif > - > static int xxs1500_pcmcia_init(struct pcmcia_init *init) > { > return PCMCIA_NUM_SOCKS; > @@ -143,13 +137,13 @@ xxs1500_pcmcia_configure_socket(const struct > pcmcia_configure *configure) > > if(configure->sock > PCMCIA_MAX_SOCK) return -1; > > - DEBUG("Vcc %dV Vpp %dV, reset %d\n", > + pr_debug("Vcc %dV Vpp %dV, reset %d\n", > configure->vcc, configure->vpp, configure->reset); > You're still changing the semantics here. The DEBUG() does __FUNCTION__ prefixing, while pr_debug() does not. This should be something like pr_debug("%s: ", __func__, ...); instead, if you want to maintain consistency. Beyond that, this looks fine, yes. - To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c
Paul Mundt wrote: > On Fri, Jan 11, 2008 at 10:45:30AM +0100, Roel Kluin wrote: >> Paul Mundt wrote: >>> On Fri, Jan 11, 2008 at 04:09:45AM +0100, Peter Stuge wrote: On Thu, Jan 10, 2008 at 10:03:58PM +0100, Roel Kluin wrote: > -#define DEBUG(x,args...) printk(__FUNCTION__ ": " x,##args) > +#define DEBUG(x, args...)printk("%s: ", __func__, x, ##args) Can this really be expected to work when x contains conversions? How about: #define DEBUG(x, args...) printk("%s: " x, __func__, ##args) >>> How about throwing out hand-rolled debug printk wrappers for the >>> brain-damage they are and using the ones the kernel provides instead? >>> >> Should it be done like this? >> -- >> Replace printk wrapper - with a syntax error - by pr_debug >> >> Signed-off-by: Roel Kluin <[EMAIL PROTECTED]> > > Close. But in this case #define DEBUG is already instrumented by the > subsystem-wide debug option if it's enabled, so it's preferable to use > that and just drop the special debug Kconfig option completely. > > Take a look at how CONFIG_PCMCIA_DEBUG is handled. In drivers/pcmcia/Makefile, when CONFIG_PCMCIA_DEBUG=y, it gives EXTRA_CFLAGS += -DDEBUG which causes the definition of DEBUG as a macro, with definition 1. > With DEBUG()->pr_debug() conversion here you've silently dropped the > __func__ prefixing. Note that dev_dbg() is usually preferred when you can > get a hold of a struct device pointer, as it takes care of prettifying > the output with the driver name and so on, rather than the convention of > adding a prefix. If you can't get at the struct device pointer, you'll > probably just want to insert the __func__ prefixing manually at the > callsites. Ah, ok, then this should be right: -- Replace printk wrapper - with a syntax error - by pr_debug. DEBUG is defined 1 when CONFIG_PCMCIA_DEBUG is set. Signed-off-by: Roel Kluin <[EMAIL PROTECTED]> --- diff --git a/drivers/pcmcia/au1000_xxs1500.c b/drivers/pcmcia/au1000_xxs1500.c index ce9d5c4..8e6426b 100644 --- a/drivers/pcmcia/au1000_xxs1500.c +++ b/drivers/pcmcia/au1000_xxs1500.c @@ -55,12 +55,6 @@ #define PCMCIA_NUM_SOCKS (PCMCIA_MAX_SOCK + 1) #define PCMCIA_IRQ AU1000_GPIO_4 -#if 0 -#define DEBUG(x,args...) printk(__FUNCTION__ ": " x,##args) -#else -#define DEBUG(x,args...) -#endif - static int xxs1500_pcmcia_init(struct pcmcia_init *init) { return PCMCIA_NUM_SOCKS; @@ -143,13 +137,13 @@ xxs1500_pcmcia_configure_socket(const struct pcmcia_configure *configure) if(configure->sock > PCMCIA_MAX_SOCK) return -1; - DEBUG("Vcc %dV Vpp %dV, reset %d\n", + pr_debug("Vcc %dV Vpp %dV, reset %d\n", configure->vcc, configure->vpp, configure->reset); switch(configure->vcc){ case 33: /* Vcc 3.3V */ /* turn on power */ - DEBUG("turn on power\n"); + pr_debug("turn on power\n"); au_writel((au_readl(GPIO2_PINSTATE) & ~(1<<14))|(1<<30), GPIO2_OUTPUT); au_sync_delay(100); @@ -166,7 +160,7 @@ xxs1500_pcmcia_configure_socket(const struct pcmcia_configure *configure) } if (!configure->reset) { - DEBUG("deassert reset\n"); + pr_debug("deassert reset\n"); au_writel((au_readl(GPIO2_PINSTATE) & ~(1<<4))|(1<<20), GPIO2_OUTPUT); au_sync_delay(100); @@ -174,7 +168,7 @@ xxs1500_pcmcia_configure_socket(const struct pcmcia_configure *configure) GPIO2_OUTPUT); } else { - DEBUG("assert reset\n"); + pr_debug("assert reset\n"); au_writel(au_readl(GPIO2_PINSTATE) | (1<<4)|(1<<20), GPIO2_OUTPUT); } - To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c
On Fri, Jan 11, 2008 at 10:45:30AM +0100, Roel Kluin wrote: > Paul Mundt wrote: > > On Fri, Jan 11, 2008 at 04:09:45AM +0100, Peter Stuge wrote: > >> On Thu, Jan 10, 2008 at 10:03:58PM +0100, Roel Kluin wrote: > >>> -#define DEBUG(x,args...) printk(__FUNCTION__ ": " x,##args) > >>> +#define DEBUG(x, args...)printk("%s: ", __func__, x, ##args) > >> Can this really be expected to work when x contains conversions? > >> > >> How about: > >> > >> #define DEBUG(x, args...) printk("%s: " x, __func__, ##args) > >> > > How about throwing out hand-rolled debug printk wrappers for the > > brain-damage they are and using the ones the kernel provides instead? > > > Should it be done like this? > -- > Replace printk wrapper - with a syntax error - by pr_debug > > Signed-off-by: Roel Kluin <[EMAIL PROTECTED]> Close. But in this case #define DEBUG is already instrumented by the subsystem-wide debug option if it's enabled, so it's preferable to use that and just drop the special debug Kconfig option completely. Take a look at how CONFIG_PCMCIA_DEBUG is handled. With DEBUG()->pr_debug() conversion here you've silently dropped the __func__ prefixing. Note that dev_dbg() is usually preferred when you can get a hold of a struct device pointer, as it takes care of prettifying the output with the driver name and so on, rather than the convention of adding a prefix. If you can't get at the struct device pointer, you'll probably just want to insert the __func__ prefixing manually at the callsites. - To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c
Paul Mundt wrote: > On Fri, Jan 11, 2008 at 04:09:45AM +0100, Peter Stuge wrote: >> On Thu, Jan 10, 2008 at 10:03:58PM +0100, Roel Kluin wrote: >>> -#define DEBUG(x,args...) printk(__FUNCTION__ ": " x,##args) >>> +#define DEBUG(x, args...) printk("%s: ", __func__, x, ##args) >> Can this really be expected to work when x contains conversions? >> >> How about: >> >> #define DEBUG(x, args...) printk("%s: " x, __func__, ##args) >> > How about throwing out hand-rolled debug printk wrappers for the > brain-damage they are and using the ones the kernel provides instead? > Should it be done like this? -- Replace printk wrapper - with a syntax error - by pr_debug Signed-off-by: Roel Kluin <[EMAIL PROTECTED]> --- diff --git a/drivers/pcmcia/au1000_xxs1500.c b/drivers/pcmcia/au1000_xxs1500.c index ce9d5c4..4c32389 100644 --- a/drivers/pcmcia/au1000_xxs1500.c +++ b/drivers/pcmcia/au1000_xxs1500.c @@ -25,6 +25,10 @@ * * */ +#ifdef CONFIG_MIPS_XXS1500_DEBUG +#define DEBUG 1 +#endif + #include #include #include @@ -55,11 +59,6 @@ #define PCMCIA_NUM_SOCKS (PCMCIA_MAX_SOCK + 1) #define PCMCIA_IRQ AU1000_GPIO_4 -#if 0 -#define DEBUG(x,args...) printk(__FUNCTION__ ": " x,##args) -#else -#define DEBUG(x,args...) -#endif static int xxs1500_pcmcia_init(struct pcmcia_init *init) { @@ -143,13 +142,13 @@ xxs1500_pcmcia_configure_socket(const struct pcmcia_configure *configure) if(configure->sock > PCMCIA_MAX_SOCK) return -1; - DEBUG("Vcc %dV Vpp %dV, reset %d\n", + pr_debug("Vcc %dV Vpp %dV, reset %d\n", configure->vcc, configure->vpp, configure->reset); switch(configure->vcc){ case 33: /* Vcc 3.3V */ /* turn on power */ - DEBUG("turn on power\n"); + pr_debug("turn on power\n"); au_writel((au_readl(GPIO2_PINSTATE) & ~(1<<14))|(1<<30), GPIO2_OUTPUT); au_sync_delay(100); @@ -166,7 +165,7 @@ xxs1500_pcmcia_configure_socket(const struct pcmcia_configure *configure) } if (!configure->reset) { - DEBUG("deassert reset\n"); + pr_debug("deassert reset\n"); au_writel((au_readl(GPIO2_PINSTATE) & ~(1<<4))|(1<<20), GPIO2_OUTPUT); au_sync_delay(100); @@ -174,7 +173,7 @@ xxs1500_pcmcia_configure_socket(const struct pcmcia_configure *configure) GPIO2_OUTPUT); } else { - DEBUG("assert reset\n"); + pr_debug("assert reset\n"); au_writel(au_readl(GPIO2_PINSTATE) | (1<<4)|(1<<20), GPIO2_OUTPUT); } - To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c
On Fri, Jan 11, 2008 at 04:09:45AM +0100, Peter Stuge wrote: > On Thu, Jan 10, 2008 at 10:03:58PM +0100, Roel Kluin wrote: > > -#define DEBUG(x,args...) printk(__FUNCTION__ ": " x,##args) > > +#define DEBUG(x, args...) printk("%s: ", __func__, x, ##args) > > Can this really be expected to work when x contains conversions? > > How about: > > #define DEBUG(x, args...) printk("%s: " x, __func__, ##args) > How about throwing out hand-rolled debug printk wrappers for the brain-damage they are and using the ones the kernel provides instead? dev_dbg() and pr_debug() both manage to get these semantics right, and you can even bury the #define DEBUG underneath some Kconfig silliness if you're the kind of person that leans that way. Maybe we can just amend checkpatch to delete a patch out of protest if it introduces printk() wrappers.. - To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c
On Fri, Jan 11, 2008 at 12:42:40PM +0900, Paul Mundt wrote: > How about throwing out hand-rolled debug printk wrappers for the > brain-damage they are and using the ones the kernel provides > instead? Sounds great! //Peter - To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c
On Thu, Jan 10, 2008 at 10:03:58PM +0100, Roel Kluin wrote: > -#define DEBUG(x,args...) printk(__FUNCTION__ ": " x,##args) > +#define DEBUG(x, args...)printk("%s: ", __func__, x, ##args) Can this really be expected to work when x contains conversions? How about: #define DEBUG(x, args...) printk("%s: " x, __func__, ##args) //Peter - To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c
Al Viro wrote: > __func__ is C99, but it's not what __FUNCTION__ used to be - it's not a > string literal. 6.4.2.2(1): > > The identifier __func__ shall be implicitly declared by the translator > as if, immediately following the opening brace of each function definition, > the declaration > static const char __func__[] = "function-name"; > appeared, where function-name is the name of the lexically-enclosing function. > > IOW, it's a phase 7 (parsing and translation of translation units) and not > phase 4 (preprocessor). Practical implications are: > * _way_ fewer kludges > * it happens after phase 6 (string literal concatenation) > So __FUNCION__ " is called" within body of foo() would result in > "foo is called" while __func__ "is called" is a syntax error. > > These days old gcc __FUNCTION__ is gone; it's a macro expanding to __func__, > so behaviour does *not* match the original (see above). so if I understand correctly, this requires a fix: -- __FUNCTION__ is a macro expanding to __func__ and translated as if previously in that function was declared: static const char __func__[] = "function-name"; As is DEBUG() - when active - will produce a syntax error. Signed-off-by: Roel Kluin <[EMAIL PROTECTED]> --- diff --git a/drivers/pcmcia/au1000_xxs1500.c b/drivers/pcmcia/au1000_xxs1500.c index ce9d5c4..855e1d6 100644 --- a/drivers/pcmcia/au1000_xxs1500.c +++ b/drivers/pcmcia/au1000_xxs1500.c @@ -56,7 +56,7 @@ #define PCMCIA_IRQ AU1000_GPIO_4 #if 0 -#define DEBUG(x,args...) printk(__FUNCTION__ ": " x,##args) +#define DEBUG(x, args...) printk("%s: ", __func__, x, ##args) #else #define DEBUG(x,args...) #endif - To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c
On Fri, Jan 04, 2008 at 09:12:28PM -0700, Andreas Dilger wrote: > What's wrong with __FUNCTION__? I thought that was ANSI C? __FUNCTION__ is a gccism of dubious taste - it pretends to be a macro when it's something far out of scope of preprocessor. Think for a minute and you'll see why - you can't expand it until you are done with parsing. __func__ is C99, but it's not what __FUNCTION__ used to be - it's not a string literal. 6.4.2.2(1): The identifier __func__ shall be implicitly declared by the translator as if, immediately following the opening brace of each function definition, the declaration static const char __func__[] = "function-name"; appeared, where function-name is the name of the lexically-enclosing function. IOW, it's a phase 7 (parsing and translation of translation units) and not phase 4 (preprocessor). Practical implications are: * _way_ fewer kludges * it happens after phase 6 (string literal concatenation) So __FUNCION__ " is called" within body of foo() would result in "foo is called" while __func__ "is called" is a syntax error. These days old gcc __FUNCTION__ is gone; it's a macro expanding to __func__, so behaviour does *not* match the original (see above). The thing is, it's not something like __FILE__ or __LINE__ and never had been; it tried to pretend that it had been like those but that required far too nasty kludges and these days it is firmly in the "nothing to do with preprocessor" land. Old name is #defined to __func__ to approximate the old behaviour, but it doesn't approximate it all that well and it's really not fit for anything but backwards compatibility in legacy code. BTW, we used to have code that broke when gcc abandoned the old kludge - several years ago there'd been a pile of patches fixing the resulting turds. - To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c
Andreas Dilger пишет: > On Jan 04, 2008 14:41 +0100, Richard Knutsson wrote: >>> @@ -54,6 +54,6 @@ int __ext4_journal_dirty_metadata(const char *where, >>> { >>> int err = jbd2_journal_dirty_metadata(handle, bh); >>> if (err) >>> - ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err); >>> + ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err); >>> return err; >>> } >> What about changing the __FUNCTION__ to __func__, while you are at it? > > What's wrong with __FUNCTION__? I thought that was ANSI C? No, it was not. The ANSI C 1990 Standard defines the following so-called "predefined macros": __LINE__, __FILE__, __DATE__, __TIME__, and __STDC__. The ISO/IEC 9899 Standard commonly referred to as the C99, defines a few additional predefined macros, as well as an additional predefined identifier __func__. For more information please refer to the ISO/IEC 9899 document itself, which is freely available for download at the time of me writing this. Although seemingly "natural", the __FUNCTION__ macro has never been part of the C Standard. Dmitri > > Cheers, Andreas > -- > Andreas Dilger > Sr. Staff Engineer, Lustre Group > Sun Microsystems of Canada, Inc. > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to [EMAIL PROTECTED] > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > - To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c
Andreas Dilger пишет: > On Jan 04, 2008 14:41 +0100, Richard Knutsson wrote: >>> @@ -54,6 +54,6 @@ int __ext4_journal_dirty_metadata(const char *where, >>> { >>> int err = jbd2_journal_dirty_metadata(handle, bh); >>> if (err) >>> - ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err); >>> + ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err); >>> return err; >>> } >> What about changing the __FUNCTION__ to __func__, while you are at it? > > What's wrong with __FUNCTION__? I thought that was ANSI C? No, it was not. The ANSI C 1990 Standard defines the following so-called "predefined macros": __LINE__, __FILE__, __DATE__, __TIME__, and __STDC__. The ISO/IEC 9899 Standard commonly referred to as the C99, defines a few additional predefined macros, as well as an additional predefined identifier __func__. For more information please refer to the ISO/IEC 9899 document itself, which is freely available for download at the time of me writing this. Although seemingly "natural", the __FUNCTION__ macro has never been part of the C Standard. Dmitri > > Cheers, Andreas > -- > Andreas Dilger > Sr. Staff Engineer, Lustre Group > Sun Microsystems of Canada, Inc. > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to [EMAIL PROTECTED] > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > - To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c
On Jan 04, 2008 14:41 +0100, Richard Knutsson wrote: >> @@ -54,6 +54,6 @@ int __ext4_journal_dirty_metadata(const char *where, >> { >> int err = jbd2_journal_dirty_metadata(handle, bh); >> if (err) >> -ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err); >> +ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err); >> return err; >> } > > What about changing the __FUNCTION__ to __func__, while you are at it? What's wrong with __FUNCTION__? I thought that was ANSI C? 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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c
Vous m'avez dit récemment : [snip] > Hi Mathieu > > What about changing the __FUNCTION__ to __func__, while you are at it? well, won't do any harm, even though other compilers than GCC are not officialy supported :) thanks a lot And I think I will revamp, including fixes into fs/ext2 -- Mathieu - To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c
Mathieu Segaud wrote: Misc style fixes from checkpatch.pl Signed-off-by: Mathieu Segaud <[EMAIL PROTECTED]> --- fs/ext3/ext3_jbd.c | 12 ++-- fs/ext4/ext4_jbd2.c | 12 ++-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/fs/ext3/ext3_jbd.c b/fs/ext3/ext3_jbd.c index e1f91fd..6c96260 100644 --- a/fs/ext3/ext3_jbd.c +++ b/fs/ext3/ext3_jbd.c @@ -9,7 +9,7 @@ int __ext3_journal_get_undo_access(const char *where, handle_t *handle, { int err = journal_get_undo_access(handle, bh); if (err) - ext3_journal_abort_handle(where, __FUNCTION__, bh, handle,err); + ext3_journal_abort_handle(where, __FUNCTION__, bh, handle, err); return err; } @@ -18,7 +18,7 @@ int __ext3_journal_get_write_access(const char *where, handle_t *handle, { int err = journal_get_write_access(handle, bh); if (err) - ext3_journal_abort_handle(where, __FUNCTION__, bh, handle,err); + ext3_journal_abort_handle(where, __FUNCTION__, bh, handle, err); return err; } @@ -27,7 +27,7 @@ int __ext3_journal_forget(const char *where, handle_t *handle, { int err = journal_forget(handle, bh); if (err) - ext3_journal_abort_handle(where, __FUNCTION__, bh, handle,err); + ext3_journal_abort_handle(where, __FUNCTION__, bh, handle, err); return err; } @@ -36,7 +36,7 @@ int __ext3_journal_revoke(const char *where, handle_t *handle, { int err = journal_revoke(handle, blocknr, bh); if (err) - ext3_journal_abort_handle(where, __FUNCTION__, bh, handle,err); + ext3_journal_abort_handle(where, __FUNCTION__, bh, handle, err); return err; } @@ -45,7 +45,7 @@ int __ext3_journal_get_create_access(const char *where, { int err = journal_get_create_access(handle, bh); if (err) - ext3_journal_abort_handle(where, __FUNCTION__, bh, handle,err); + ext3_journal_abort_handle(where, __FUNCTION__, bh, handle, err); return err; } @@ -54,6 +54,6 @@ int __ext3_journal_dirty_metadata(const char *where, { int err = journal_dirty_metadata(handle, bh); if (err) - ext3_journal_abort_handle(where, __FUNCTION__, bh, handle,err); + ext3_journal_abort_handle(where, __FUNCTION__, bh, handle, err); return err; } diff --git a/fs/ext4/ext4_jbd2.c b/fs/ext4/ext4_jbd2.c index d6afe4e..2256c43 100644 --- a/fs/ext4/ext4_jbd2.c +++ b/fs/ext4/ext4_jbd2.c @@ -9,7 +9,7 @@ int __ext4_journal_get_undo_access(const char *where, handle_t *handle, { int err = jbd2_journal_get_undo_access(handle, bh); if (err) - ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err); + ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err); return err; } @@ -18,7 +18,7 @@ int __ext4_journal_get_write_access(const char *where, handle_t *handle, { int err = jbd2_journal_get_write_access(handle, bh); if (err) - ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err); + ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err); return err; } @@ -27,7 +27,7 @@ int __ext4_journal_forget(const char *where, handle_t *handle, { int err = jbd2_journal_forget(handle, bh); if (err) - ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err); + ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err); return err; } @@ -36,7 +36,7 @@ int __ext4_journal_revoke(const char *where, handle_t *handle, { int err = jbd2_journal_revoke(handle, blocknr, bh); if (err) - ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err); + ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err); return err; } @@ -45,7 +45,7 @@ int __ext4_journal_get_create_access(const char *where, { int err = jbd2_journal_get_create_access(handle, bh); if (err) - ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err); + ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err); return err; } @@ -54,6 +54,6 @@ int __ext4_journal_dirty_metadata(const char *where, { int err = jbd2_journal_dirty_metadata(handle, bh); if (err) - ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err); + ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err); return err; } Hi Mathieu What about changing the __FUNCTION__ to __func__, while you are at it? Richard Knutsson - To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html