2002-08-24 23:43:10

by Luca Barbieri

[permalink] [raw]
Subject: Broken inlines all over the source tree

GCC can only inline functions when the function definition comes before
its use.
Unfortunately a lot of code hasn't been written with this in mind.

Here is a list of files containing forward declarations for inline
functions (that are either broken or redundant).
The list was made with grep -E '(inline|inline__)[ '$'\t''][^{]*;' so
it's not very accurate.

gcc -Winline would provide better results.
How about adding it to the build CFLAGS?

Here is the list (excluding mm/rmap.c that I already fixed).

./drivers/hotplug/ibmphp_res.c
./drivers/net/hamradio/dmascc.c
./drivers/net/tulip/dmfe.c
./drivers/net/au1000_eth.c
./drivers/net/wan/lmc/lmc_debug.h
./drivers/net/wan/lmc/lmc_media.c
./drivers/net/wan/dscc4.c
./drivers/net/e1000/e1000_main.c
./drivers/net/sb1000.c
./drivers/net/ioc3-eth.c
./drivers/net/seeq8005.c
./drivers/net/mace.c
./drivers/net/eql.c
./drivers/net/bonding.c
./drivers/net/via-rhine.c
./drivers/net/hamachi.c
./drivers/net/smc9194.c
./drivers/isdn/hisax/isar.c
./drivers/isdn/hisax/ipacx.c
./drivers/isdn/sc/command.c
./drivers/video/riva/fbdev.c
./drivers/video/fbcon.c
./drivers/video/cyberfb.h
./drivers/video/virgefb.c
./drivers/video/platinumfb.c
./drivers/video/valkyriefb.c
./drivers/media/video/w9966.c
./drivers/media/video/planb.c
./drivers/media/radio/radio-maestro.c
./drivers/char/ftape/zftape/zftape-vtbl.h
./drivers/char/istallion.c
./drivers/char/stallion.c
./drivers/char/dz.h
./drivers/char/ser_a2232.c
./drivers/char/qtronix.c
./drivers/char/rtc.c
./drivers/char/epca.c
./drivers/char/pcxx.c
./drivers/char/mxser.c
./drivers/char/ip2main.c
./drivers/acorn/block/fd1772.c
./drivers/scsi/aic7xxx/aic7xxx_osm.h
./drivers/scsi/aic7xxx/aic7xxx_linux.c
./drivers/scsi/aic7xxx/aic7xxx_inline.h
./drivers/scsi/ips.c
./drivers/scsi/sun3_scsi.c
./drivers/scsi/sim710.c
./drivers/scsi/advansys.c
./drivers/scsi/fastlane.c
./drivers/scsi/sg.c
./drivers/scsi/AM53C974.c
./drivers/scsi/mac_esp.c
./drivers/scsi/tmscsim.c
./drivers/scsi/ultrastor.c
./drivers/scsi/megaraid.h
./drivers/scsi/qla1280.c
./drivers/s390/char/tape.c
./drivers/s390/char/tape.h
./drivers/mtd/devices/doc1000.c
./drivers/usb/media/se401.h
./drivers/usb/serial/whiteheat.c
./drivers/usb/net/rtl8150.c
./drivers/usb/host/hc_simple.h
./drivers/block/cpqarray.c
./drivers/block/ataflop.c
./drivers/block/xd.h
./drivers/block/cciss.c
./drivers/sgi/char/ds1286.c
./drivers/cdrom/sonycd535.c
./arch/mips/au1000/common/irq.c
./arch/ia64/sn/io/l1.c
./include/net/irda/timer.h
./include/net/ip.h
./include/linux/fsfilter.h
./include/linux/elevator.h
./include/linux/sched.h
./include/linux/coda_linux.h
./include/linux/blkdev.h
./include/linux/intermezzo_fs.h
./include/linux/bio.h
./include/linux/reiserfs_fs.h
./include/asm-ppc64/tlb.h
./include/asm-ia64/unistd.h
./include/asm-x86_64/apic.h
./include/asm-s390x/processor.h
./include/asm-arm/thread_info.h
./include/asm-arm/current.h
./include/asm-arm/unistd.h
./include/asm-i386/apic.h
./net/core/sock.c
./net/irda/wrapper.c
./net/bluetooth/sco.c
./net/bluetooth/l2cap.c
./net/atm/lec.c
./net/wanrouter/af_wanpipe.c
./net/appletalk/ddp.c
./fs/jfs/resize.c
./fs/ntfs/ntfs.h
./fs/intermezzo/super.c
./fs/freevxfs/vxfs_extern.h
./fs/affs/file.c


Attachments:
signature.asc (189.00 B)
This is a digitally signed message part

2002-08-25 08:01:16

by Russell King

[permalink] [raw]
Subject: Re: Broken inlines all over the source tree

On Sun, Aug 25, 2002 at 01:47:18AM +0200, Luca Barbieri wrote:
> ./drivers/acorn/block/fd1772.c

False positive; set_head_settle_flag is declared with code before use.
False positive; get_head_settle_flag is declared with code before use.
Correct positive; copy_buffer needs fixing, thanks for finding that.

> ./include/asm-arm/thread_info.h

False positive; current_thread_info is declared with code before use.

> ./include/asm-arm/current.h

False positive; get_current is declared with code before use.

> ./include/asm-arm/unistd.h

False positive; _syscall3 is a macro containing code.

--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html

2002-08-25 10:13:01

by Roger Luethi

[permalink] [raw]
Subject: Re: Broken inlines all over the source tree

On Sun, 25 Aug 2002 01:47:18 +0200, Luca Barbieri wrote:
> ./drivers/net/via-rhine.c

via_restart_tx() was already defined before use.
clear_tally_counters() is now fixed in my tree.

Thx.

2002-08-26 07:34:04

by David Miller

[permalink] [raw]
Subject: Re: Broken inlines all over the source tree

From: Luca Barbieri <[email protected]>
Date: 25 Aug 2002 01:47:18 +0200

./include/net/ip.h

False positive, ip_finish_output is merely declared extern here with
the __inline__ attribute so that the functions args+attributes match
with what the real function definition has in net/ipv4/ip_output.c

We don't intend it to get inlined when called from other files :-)

./net/core/sock.c

ROFL, maybe your script is matching sk->urginline :-)
I can't find anything else in this file.

Franks a lot,
David S. Miller
[email protected]

2002-08-26 16:18:02

by Greg KH

[permalink] [raw]
Subject: Re: Broken inlines all over the source tree

On Sun, Aug 25, 2002 at 01:47:18AM +0200, Luca Barbieri wrote:
> ./drivers/usb/media/se401.h

Should be fixed, Jeroen, do you want to do this?

> ./drivers/usb/serial/whiteheat.c

False positive, those functions are never even called :)

> ./drivers/usb/net/rtl8150.c

Should be fixed, Petko want to do it?

> ./drivers/usb/host/hc_simple.h

Hm, these also need to be fixed, but there doesn't seem to be a
maintainer for the code. I'll just take the inline marking off of them,
if no one minds.

thanks,

greg k-h

2002-08-26 16:35:25

by Luca Barbieri

[permalink] [raw]
Subject: Re: Broken inlines all over the source tree

> > ./drivers/usb/serial/whiteheat.c
>
> False positive, those functions are never even called :)
Remove them :)

> > ./drivers/usb/host/hc_simple.h
>
> Hm, these also need to be fixed, but there doesn't seem to be a
> maintainer for the code. I'll just take the inline marking off of them,
> if no one minds.
Or you could fix them by removing the declarations and moving the
definitions where the declarations were (if they use something declared
between the declaration and the definition more changes are required).


Attachments:
signature.asc (189.00 B)
This is a digitally signed message part

2002-08-27 06:43:09

by Petko Manolov

[permalink] [raw]
Subject: Re: Broken inlines all over the source tree

> > ./drivers/usb/net/rtl8150.c
>
> Should be fixed, Petko want to do it?

Will send you a patch soon.


Petko

2002-08-28 20:15:27

by Jeroen Vreeken

[permalink] [raw]
Subject: Re: Broken inlines all over the source tree

On 2002.08.26 18:22:05 +0200 Greg KH wrote:
> On Sun, Aug 25, 2002 at 01:47:18AM +0200, Luca Barbieri wrote:
> > ./drivers/usb/media/se401.h
>
> Should be fixed, Jeroen, do you want to do this?

Ok, already located the cause, I will check the other inlines to and send a
patch.

Jeroen


2002-09-03 15:24:57

by Jeroen Vreeken

[permalink] [raw]
Subject: Re: Broken inlines all over the source tree

On 2002.08.26 18:22:05 +0200 Greg KH wrote:
> On Sun, Aug 25, 2002 at 01:47:18AM +0200, Luca Barbieri wrote:
> > ./drivers/usb/media/se401.h
>
> Should be fixed, Jeroen, do you want to do this?

Attached is a patch against 2.4.20-pre5 that updates the se401 driver to
update it to version 0.24.
This fixes the inline problem, a memory leak on disconnect and disables the
button for cameras that don't support it.

I haven't been folowing 2.5 for a while, but I think it will apply without
problems.

Jeroen


Attachments:
se401-0.24.diff (5.34 kB)

2002-09-04 18:14:47

by Greg KH

[permalink] [raw]
Subject: Re: Broken inlines all over the source tree

On Tue, Sep 03, 2002 at 05:33:47PM +0200, Jeroen Vreeken wrote:
> On 2002.08.26 18:22:05 +0200 Greg KH wrote:
> > On Sun, Aug 25, 2002 at 01:47:18AM +0200, Luca Barbieri wrote:
> > > ./drivers/usb/media/se401.h
> >
> > Should be fixed, Jeroen, do you want to do this?
>
> Attached is a patch against 2.4.20-pre5 that updates the se401 driver to
> update it to version 0.24.
> This fixes the inline problem, a memory leak on disconnect and disables the
> button for cameras that don't support it.
>
> I haven't been folowing 2.5 for a while, but I think it will apply without
> problems.

Sorry, but the patch does not apply (even after adjusting the path). I
get lots of failed hunks.

thanks,

greg k-h

2002-09-05 21:59:26

by Jeroen Vreeken

[permalink] [raw]
Subject: Re: Broken inlines all over the source tree

On 2002.09.04 20:17:23 +0200 Greg KH wrote:
> On Tue, Sep 03, 2002 at 05:33:47PM +0200, Jeroen Vreeken wrote:
> > On 2002.08.26 18:22:05 +0200 Greg KH wrote:
> > > On Sun, Aug 25, 2002 at 01:47:18AM +0200, Luca Barbieri wrote:
> > > > ./drivers/usb/media/se401.h
> > >
> > > Should be fixed, Jeroen, do you want to do this?
> >
> > Attached is a patch against 2.4.20-pre5 that updates the se401 driver
> to
> > update it to version 0.24.
> > This fixes the inline problem, a memory leak on disconnect and disables
> the
> > button for cameras that don't support it.
> >
> > I haven't been folowing 2.5 for a while, but I think it will apply
> without
> > problems.
>
> Sorry, but the patch does not apply (even after adjusting the path). I
> get lots of failed hunks.

I made a new patch against 2.5.33, it was mostly litle things like the v4l
changes.

Jeroen


Attachments:
se401-0.24-2.5.33.diff (5.27 kB)

2002-09-09 21:18:18

by Greg KH

[permalink] [raw]
Subject: Re: Broken inlines all over the source tree

On Fri, Sep 06, 2002 at 12:08:32AM +0200, Jeroen Vreeken wrote:
>
> I made a new patch against 2.5.33, it was mostly litle things like the v4l
> changes.

This works fine, applied.

thanks,

greg k-h