Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-11 Thread Roel Kluin 
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-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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-11 Thread Paul Mundt 
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-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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-11 Thread Roel Kluin 
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-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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-11 Thread Paul Mundt 
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-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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-11 Thread Roel Kluin 
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-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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-11 Thread Paul Mundt 
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-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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-11 Thread Roel Kluin 
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 linux/module.h
 #include linux/init.h
 #include linux/delay.h
@@ -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)  ~(114))|(130),
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)  ~(14))|(120),
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) | (14)|(120),
GPIO2_OUTPUT);
}

--
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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-11 Thread Paul Mundt 
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-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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-11 Thread Roel Kluin 
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)  ~(114))|(130),
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)  ~(14))|(120),
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) | (14)|(120),
GPIO2_OUTPUT);
}
--
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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-11 Thread Roel Kluin 
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)  ~(114))|(130),
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)  ~(14))|(120),
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) | (14)|(120),
GPIO2_OUTPUT);
}



--
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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-10 Thread Peter Stuge 
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-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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-10 Thread Paul Mundt 
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-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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-10 Thread Peter Stuge 
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-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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-10 Thread Roel Kluin 
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-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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-10 Thread Roel Kluin 
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-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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-10 Thread Peter Stuge 
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-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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-10 Thread Paul Mundt 
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-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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-10 Thread Peter Stuge 
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-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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-04 Thread Al Viro 
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-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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-04 Thread Dmitri Vorobiev 
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-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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-04 Thread Dmitri Vorobiev 
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-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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-04 Thread 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?

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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-04 Thread Mathieu SEGAUD 
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-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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-04 Thread Richard Knutsson 

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-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/

[PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-04 Thread Mathieu Segaud 
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;
 }
-- 
1.5.3.7

--
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/

[PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-04 Thread Mathieu Segaud 
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;
 }
-- 
1.5.3.7

--
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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-04 Thread Richard Knutsson 

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-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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-04 Thread Mathieu SEGAUD 
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-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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-04 Thread 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?

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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-04 Thread Dmitri Vorobiev 
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-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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-04 Thread Dmitri Vorobiev 
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-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/

Re: [PATCH] [Coding Style]: fs/ext{3,4}/ext{3,4}_jbd{,2}.c

2008-01-04 Thread Al Viro 
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-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/