__FUNCTION__ - patch for USB

__FUNCTION__ - patch for USB

[Vladimir Kondratiev]

[-- Attachment #1: Type: text/plain, Size: 810 bytes --]

Hello,
Since I have started this thread, I feel I have to do something real. 
Dummy technical work, but someone have to do it, right?
I patched USB subsystem, it uses __FUNCTION__ in deprecated way no more.
What is changed?
- in usb.h, I modified dbg(), warn(), err(), info() macros to contain 
function name in the prefix. These macros are for simple one-line messages.
- in all files under drivers/usb, all macros mentioned fixed.
- all __FUNCTION__ occurencies in drivers/usb revised.

I compiled kednel with all USB modules enabled. Since everything 
compiles OK and all changes are in message formats, this patch should 
not corrupt anything. In the worst case you will get badly formatted 
message.

Patch is against 2.4.17

Comments?

Please, when replying, CC me: mailto:vladimir.kondratiev@intel.com


[-- Attachment #2: __FUNCTION__.usb.patch.gz --]
[-- Type: application/x-gzip, Size: 38773 bytes --]

Re: __FUNCTION__ - patch for USB

[Greg KH]

On Thu, Jan 10, 2002 at 12:12:29AM +0200, Vladimir Kondratiev wrote:
> Hello,
> Since I have started this thread, I feel I have to do something real. 
> Dummy technical work, but someone have to do it, right?
> I patched USB subsystem, it uses __FUNCTION__ in deprecated way no more.
> What is changed?
> - in usb.h, I modified dbg(), warn(), err(), info() macros to contain 
> function name in the prefix. These macros are for simple one-line messages.
> - in all files under drivers/usb, all macros mentioned fixed.
> - all __FUNCTION__ occurencies in drivers/usb revised.
> 
> I compiled kednel with all USB modules enabled. Since everything 
> compiles OK and all changes are in message formats, this patch should 
> not corrupt anything. In the worst case you will get badly formatted 
> message.

Your patch makes whitespace changes to a lot of dbg() statements, but
does not modify their contents.  Can you please change this, as this
change does not need to happen.

> Patch is against 2.4.17

2.4.18-pre2 has a _lot_ of usb changes and this patch misses a number of
places.

I'd also like to see this against the 2.5.x tree first, as the
recommended compiler for the 2.4.x tree is still 2.95.3, and I don't
think that will change anytime soon.

thanks,

greg k-h

Re: __FUNCTION__ - patch for USB

[Vladimir Kondratiev]

[-- Attachment #1: Type: text/plain, Size: 1287 bytes --]

Greg KH wrote:

>
>Your patch makes whitespace changes to a lot of dbg() statements, but
>does not modify their contents.  Can you please change this, as this
>change does not need to happen.
>
Yes, I realized it, but unfortunately too late. It's hard to revise all 
patch chunk by chunk and undo changes when content not changed.

>>Patch is against 2.4.17
>>
>
>2.4.18-pre2 has a _lot_ of usb changes and this patch misses a number of
>places.
>
>I'd also like to see this against the 2.5.x tree first, as the
>recommended compiler for the 2.4.x tree is still 2.95.3, and I don't
>think that will change anytime soon.
>
>thanks,
>
>greg k-h
>
Patch against 2.4.18-pre2 attached. For 2.5 tree - wait a bit, I have to 
return for a moment to business I get salary for.

Nick Craig-Wood wrote:

>
>I was going to suggest you use the C99 __func__ rather than
>__FUNCTION__ but after a quick test it doesn't seem to be supported by
>egcs-2.91.66 so I guess that is out for the time being?  It is
>supported by gcc-2.95 though.
>
I considered it too. Unfortunately, __func__ is not supported by all gcc 
versions, so __FUNCTION__ seems to be best solution so far. Also, I 
aggregated all changes to one place in macro definition, it should be 
easier to change, should this need arise.

>



[-- Attachment #2: __FUNCTION__.usb.patch.gz --]
[-- Type: application/x-gzip, Size: 41740 bytes --]

Re: __FUNCTION__ - patch for USB

[Greg KH]

On Thu, Jan 10, 2002 at 12:52:57PM +0200, Vladimir Kondratiev wrote:
> Patch against 2.4.18-pre2 attached. For 2.5 tree - wait a bit, I have to 
> return for a moment to business I get salary for.

You still are changing a few dbg() macros that you don't have to change.

Also, info(), warn() and err() should not have __FUNCTION__ added to
them.  Have you tried running the usb code with this patch?  The USB
group gets enough grief about all of the kernel log messages that we
spit out.  We do not need to see the function name for every message we
write (the user does not need it.)

I think I'll wait for the debug level messages cleanup in the 2.5 USB
code to make this kind of change (as talked about in a previous
message.)

thanks,

greg k-h

Re: __FUNCTION__ - patch for USB

[Vladimir Kondratiev]

Greg KH wrote:

>You still are changing a few dbg() macros that you don't have to change.
>
Guilty. Goal was to force some common style. There are mix of function 
call styles like f() and f (). I suppose f() (without space between 
function name and parenthes) is better, but it is of course religious 
issue and have no 'proper' solution.

>
>Also, info(), warn() and err() should not have __FUNCTION__ added to
>them.  Have you tried running the usb code with this patch?  The USB
>group gets enough grief about all of the kernel log messages that we
>spit out.  We do not need to see the function name for every message we
>write (the user does not need it.)
>
Questionable. There are lots of calls that used to have __FUNCTION__, 
ex. 1-st chunks from patch are:

--- linux-2.4.18-pre2.orig/drivers/usb/CDCEther.c       Fri Dec 21 
19:41:55 2001
+++ linux-2.4.18-pre2.patched/drivers/usb/CDCEther.c    Thu Jan 10 
10:28:21 2002
-               warn( __FUNCTION__ " failed submint rx_urb %d", res);
+               warn("failed submint rx_urb %d", res);
-               err( "write_bulk_callback: device not running" );
+               err( "device not running" );

It is hard to be both flexible and easy. I suppose always have function 
name in the log is a good idea. Maybe, I would even add line number. 
Anyway, original macros used to add __FILE__ prefix. All these stuff 
goes to syslog, and user usually do not see it at all. If one wants to 
browse /var/log/messages, he is supposed to understand a little what he 
going to see, and function names may help.
In places where great flexibility is required, I'd propose to keep using 
printk().

>
>I think I'll wait for the debug level messages cleanup in the 2.5 USB
>code to make this kind of change (as talked about in a previous
>message.)
>
>thanks,
>
>greg k-h
>
What about macros for simple one line messages for generic usage? Any 
thoughts?
I propose the following messages format:

<module>:<file>:<line>:<function> - <message_text>

Corresponded macro would be, for example:

#ifdef MODULE
#define err(format, arg...) printk(KERN_ERR "%s:%s:%d:%s - " format "\n" 
,THIS_MODULE->name ,__FILE__, __LINE__, __FUNCTION__, ## arg)
#else
#define err(format, arg...) printk(KERN_ERR "%s:%d:%s - " format "\n" , 
__FILE__, __LINE__, __FUNCTION__, ## arg)
#endif

Re: __FUNCTION__ - patch for USB

[Nick Craig-Wood]

On Thu, Jan 10, 2002 at 12:12:29AM +0200, Vladimir Kondratiev wrote:
> Since I have started this thread, I feel I have to do something real. 
> Dummy technical work, but someone have to do it, right?
> I patched USB subsystem, it uses __FUNCTION__ in deprecated way no more.

 #ifdef DEBUG
-#define dbg(format, arg...) printk(KERN_DEBUG __FILE__ ": " format "\n" , ## arg)
+#define dbg(format, arg...) printk(KERN_DEBUG __FILE__ ":%s - " format "\n", __FUNCTION__, ## arg)
 #else

I was going to suggest you use the C99 __func__ rather than
__FUNCTION__ but after a quick test it doesn't seem to be supported by
egcs-2.91.66 so I guess that is out for the time being?  It is
supported by gcc-2.95 though.

-- 
Nick Craig-Wood
ncw@axis.demon.co.uk