Re: [PATCH] 2.4.17-pre7: fdomain_16x0_release undeclared

Re: [PATCH] 2.4.17-pre7: fdomain_16x0_release undeclared

[Marcelo Tosatti]

The second hunk of the your patch is already on my tree.

On Mon, 10 Dec 2001, Pavel Roskin wrote:

> Hello, Alan and all!
> 
> File drivers/scsi/fdomain.h declares a data structure FDOMAIN_16X0 using
> an undeclared function fdomain_16x0_release() in Linux 2.4.17-pre7.
> 
> This is not a problem for the kernel itself, but it breaks compilation of
> pcmcia-cs-3.1.30 because the later uses structure FDOMAIN_16X0.
> 
> The fix is trivial - declare fdomain_16x0_release() the same way as other
> non-static functions from fdomain.c.
> 
> Another partly related problem is a warning in fdomain.c:
> 
> fdomain.c: In function `fdomain_16x0_release':
> fdomain.c:2045: warning: control reaches end of non-void function
> 
> I wonder if the patches that introduce warnings should be allowed in the 
> stable branch?  Anyway, comparing with other SCSI drivers I see that 0 
> should be returned and scsi_unregister(shpnt) should be called before 
> that.
> 
> Here's the patch.  The part for fdomain.h is 100% safe and should be
> applied ASAP.  The part for fdomain.c should be safe too but I'll
> appreciate if somebody tests it on a real Future Domain controller.
> 
> ------------------------------
> --- linux.orig/drivers/scsi/fdomain.c
> +++ linux/drivers/scsi/fdomain.c
> @@ -2042,6 +2042,8 @@ int fdomain_16x0_release(struct Scsi_Hos
>  	if (shpnt->io_port && shpnt->n_io_port)
>  		release_region(shpnt->io_port, shpnt->n_io_port);
>  
> +	scsi_unregister(shpnt);
> +	return 0;
>  }
>  
>  MODULE_LICENSE("GPL");
> --- linux.orig/drivers/scsi/fdomain.h
> +++ linux/drivers/scsi/fdomain.h
> @@ -34,6 +34,7 @@
>  int        fdomain_16x0_biosparam( Disk *, kdev_t, int * );
>  int        fdomain_16x0_proc_info( char *buffer, char **start, off_t offset,
>  				   int length, int hostno, int inout );
> +int        fdomain_16x0_release(struct Scsi_Host *shpnt);
>  
>  #define FDOMAIN_16X0 { proc_info:      fdomain_16x0_proc_info,           \
>  		       detect:         fdomain_16x0_detect,              \
> ------------------------------
> 
> -- 
> Regards,
> Pavel Roskin
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

[PATCH] 2.4.17-pre7: fdomain_16x0_release undeclared

[Pavel Roskin]

Hello, Alan and all!

File drivers/scsi/fdomain.h declares a data structure FDOMAIN_16X0 using
an undeclared function fdomain_16x0_release() in Linux 2.4.17-pre7.

This is not a problem for the kernel itself, but it breaks compilation of
pcmcia-cs-3.1.30 because the later uses structure FDOMAIN_16X0.

The fix is trivial - declare fdomain_16x0_release() the same way as other
non-static functions from fdomain.c.

Another partly related problem is a warning in fdomain.c:

fdomain.c: In function `fdomain_16x0_release':
fdomain.c:2045: warning: control reaches end of non-void function

I wonder if the patches that introduce warnings should be allowed in the 
stable branch?  Anyway, comparing with other SCSI drivers I see that 0 
should be returned and scsi_unregister(shpnt) should be called before 
that.

Here's the patch.  The part for fdomain.h is 100% safe and should be
applied ASAP.  The part for fdomain.c should be safe too but I'll
appreciate if somebody tests it on a real Future Domain controller.

------------------------------
--- linux.orig/drivers/scsi/fdomain.c
+++ linux/drivers/scsi/fdomain.c
@@ -2042,6 +2042,8 @@ int fdomain_16x0_release(struct Scsi_Hos
 	if (shpnt->io_port && shpnt->n_io_port)
 		release_region(shpnt->io_port, shpnt->n_io_port);
 
+	scsi_unregister(shpnt);
+	return 0;
 }
 
 MODULE_LICENSE("GPL");
--- linux.orig/drivers/scsi/fdomain.h
+++ linux/drivers/scsi/fdomain.h
@@ -34,6 +34,7 @@
 int        fdomain_16x0_biosparam( Disk *, kdev_t, int * );
 int        fdomain_16x0_proc_info( char *buffer, char **start, off_t offset,
 				   int length, int hostno, int inout );
+int        fdomain_16x0_release(struct Scsi_Host *shpnt);
 
 #define FDOMAIN_16X0 { proc_info:      fdomain_16x0_proc_info,           \
 		       detect:         fdomain_16x0_detect,              \
------------------------------

-- 
Regards,
Pavel Roskin

Re: [PATCH] 2.4.17-pre7: fdomain_16x0_release undeclared

[Alan Cox]

> > Another partly related problem is a warning in fdomain.c:
> > 
> > fdomain.c: In function `fdomain_16x0_release':
> > fdomain.c:2045: warning: control reaches end of non-void function
> > 
> > I wonder if the patches that introduce warnings should be allowed in the 
> > stable branch?  Anyway, comparing with other SCSI drivers I see that 0 
> > should be returned and scsi_unregister(shpnt) should be called before 
> > that.

All the drivers I looked at return 1 and don't call scsi_unregister(shpnt).
Inspecting the core code shows that 

	1.	The return code is ignored (see I did test it worked 8))
	2.	scsi_unregister() is done by the core code for us

So I believe it simply needs a 

	return 1 

on the end

Re: [PATCH] 2.4.17-pre7: fdomain_16x0_release undeclared

[André Dahlqvist]

On Mon, Dec 10, 2001 at 06:01:03PM -0200, Marcelo Tosatti wrote:

[One line of text and quoted a full page or something]

No offence marcelo, but could you please stop it with the full quotes?
-- 

André Dahlqvist <andre.dahlqvist@telia.com>

Re: [PATCH] 2.4.17-pre7: fdomain_16x0_release undeclared

[Pavel Roskin]

Hi, Alan!

> > > I wonder if the patches that introduce warnings should be allowed in the 
> > > stable branch?  Anyway, comparing with other SCSI drivers I see that 0 
> > > should be returned and scsi_unregister(shpnt) should be called before 
> > > that.
> 
> All the drivers I looked at return 1 and don't call scsi_unregister(shpnt).

Strange.  That's what I see:

advansys.c:5777    scsi_unregister, return 0
eata.c:2069        scsi_unregister, return FALSE (i.e. 0)
inia100.c:790      no scsi_unregister, return 0
aha152x.c:1409     scsi_unregister, return 0
gdth.c:4373        scsi_unregister, return 0
ibmmca.c:1851      no scsi_unregister, return 0
53c7,8xx.c:6431    no scsi_unregister, return 1
eata_dma.c:146     no scsi_unregister, return 1

> Inspecting the core code shows that 
> 
> 	1.	The return code is ignored (see I did test it worked 8))
> 	2.	scsi_unregister() is done by the core code for us

Anyway, if it doesn't matter then let's not waste time on it.

> So I believe it simply needs a 
> 
> 	return 1 
> 
> on the end

Fine.  That's the updated patch:

======================================
--- linux.orig/drivers/scsi/fdomain.c
+++ linux/drivers/scsi/fdomain.c
@@ -2042,6 +2042,7 @@ int fdomain_16x0_release(struct Scsi_Hos
 	if (shpnt->io_port && shpnt->n_io_port)
 		release_region(shpnt->io_port, shpnt->n_io_port);
 
+	return 1;
 }
 
 MODULE_LICENSE("GPL");
--- linux.orig/drivers/scsi/fdomain.h
+++ linux/drivers/scsi/fdomain.h
@@ -34,6 +34,7 @@
 int        fdomain_16x0_biosparam( Disk *, kdev_t, int * );
 int        fdomain_16x0_proc_info( char *buffer, char **start, off_t offset,
 				   int length, int hostno, int inout );
+int        fdomain_16x0_release(struct Scsi_Host *shpnt);
 
 #define FDOMAIN_16X0 { proc_info:      fdomain_16x0_proc_info,           \
 		       detect:         fdomain_16x0_detect,              \
======================================

-- 
Regards,
Pavel Roskin