lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Sat, 24 Mar 2007 12:11:05 -0400
From:	Douglas Gilbert <dougg@...que.net>
To:	Adrian Bunk <bunk@...sta.de>
CC:	Andrew Morton <akpm@...ux-foundation.org>,
	James.Bottomley@...elEye.com, linux-kernel@...r.kernel.org,
	linux-scsi@...r.kernel.org
Subject: Re: [-mm patch] drivers/scsi/constants.c: make 2 functions static

Adrian Bunk wrote:
> On Mon, Mar 19, 2007 at 08:56:23PM -0800, Andrew Morton wrote:
>> ...
>> Changes since 2.6.21-rc3-mm1:
>> ...
>>  git-scsi-misc.patch
>> ...
>>  git trees
>> ...
> 
> 
> This patch makes two needlessly global functions static.
> 
> Signed-off-by: Adrian Bunk <bunk@...sta.de>
> 
> ---
> 
>  drivers/scsi/constants.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> --- linux-2.6.21-rc4-mm1/drivers/scsi/constants.c.old	2007-03-23 23:26:39.000000000 +0100
> +++ linux-2.6.21-rc4-mm1/drivers/scsi/constants.c	2007-03-23 23:26:55.000000000 +0100
> @@ -1235,7 +1235,7 @@
>  }
>  EXPORT_SYMBOL(scsi_print_sense_hdr);
>  
> -void
> +static void
>  scsi_decode_sense_buffer(const unsigned char *sense_buffer, int sense_len,
>  		       struct scsi_sense_hdr *sshdr)
>  {
> @@ -1258,7 +1258,7 @@
>  	}
>  }
>  
> -void
> +static void
>  scsi_decode_sense_extras(const unsigned char *sense_buffer, int sense_len,
>  			 struct scsi_sense_hdr *sshdr)
>  {

Adrian,
Who put those functions in?

The names and arguments look very similar to these
exported functions in scsi_error.c *** :
  scsi_normalize_sense
  scsi_sense_desc_find
  scsi_get_sense_info_fld

that I can see in 2.6.21-rc4

The proposed scsi_decode_sense_buffer() looks broken because
it can fail and should return an int reflecting that.
How scsi_decode_sense_extras() works is intriguing, unless
struct scsi_sense_hdr has been changed as well.


*** Putting sense decode logic in scsi_error.c is wrong
because:
  - the ATA command set is proposing an ATA REQUEST SENSE
    command to yield a sense buffer
  - sense buffers don't necessarily indicate errors.

So moving those functions out of scsi_error.c IMO is a
good idea. Breaking them in the move isn't.

Doug Gilbert


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ