RFC arch/sh/kernel/traps_64.c - unclear if/else construct

RFC arch/sh/kernel/traps_64.c - unclear if/else construct

[Nicholas Mc Guire]

Hi !

 scanning for  if STATEMENT else STATEMENT  triggered here - and it does look
like it needs a fix-up or at least some comments.

<snip?
int hpux_sysfs(int opcode, unsigned long arg1, unsigned long arg2)
{
	 int fstype;
	 ...
		  /* String could be altered by userspace after strlen_user() */
		  fsname[len - 1] = '\0';

		  printk(KERN_DEBUG "that is '%s' as (char *)\n", fsname);
		  if ( !strcmp(fsname, "hfs") ) {
			   fstype = 0;
		  } else {
			   fstype = 0;
		  }  
		     
		  kfree(fsname);

		  printk(KERN_DEBUG "returning fstype=%d\n", fstype);
		  return fstype; /* something other than default */
<snip>

 The if-else here has no effect and the printk will not convey any information
as its always fstype==0, finally the return statement comment indicates that it
should not be the default...

 Can't come up with a reasonable cleanup patch here - but this looks like it
needs a review.

thx!
hofrat

Re: RFC arch/sh/kernel/traps_64.c - unclear if/else construct (hpux compat layer)

[Helge Deller]

Hello Nicholas,

On 03.02.2015 10:37, Nicholas Mc Guire wrote:
>   scanning for  if STATEMENT else STATEMENT  triggered here - and it does look
> like it needs a fix-up or at least some comments.
>
> <snip?
> int hpux_sysfs(int opcode, unsigned long arg1, unsigned long arg2)
> {
> 	 int fstype;
> 	 ...
> 		  /* String could be altered by userspace after strlen_user() */
> 		  fsname[len - 1] = '\0';
>
> 		  printk(KERN_DEBUG "that is '%s' as (char *)\n", fsname);
> 		  if ( !strcmp(fsname, "hfs") ) {
> 			   fstype = 0;
> 		  } else {
> 			   fstype = 0;
> 		  }
> 		
> 		  kfree(fsname);
>
> 		  printk(KERN_DEBUG "returning fstype=%d\n", fstype);
> 		  return fstype; /* something other than default */
> <snip>
>
>   The if-else here has no effect and the printk will not convey any information
> as its always fstype==0, finally the return statement comment indicates that it
> should not be the default...
>
>   Can't come up with a reasonable cleanup patch here - but this looks like it
> needs a review.

The hpux code is broken anyway.
I'm going to remove it from the tree.

Thanks!

Helge

Re: RFC arch/sh/kernel/traps_64.c - unclear if/else construct (hpux compat layer)

[Paul Bolle]

Hi Helge,

On Mon, 2015-02-16 at 22:24 +0100, Helge Deller wrote:
> The hpux code is broken anyway.
> I'm going to remove it from the tree.

That happened in commit 04c161497716 ("parisc: hpux - Drop support for
HP-UX binaries"), which is included in today's linux-next
(next-20150217). I noticed because a script I use to check linux-next
spotted a problem with it.

That commit removed the Kconfig symbol HPUX. As a result BINFMT_SOM (see
fs/Kconfig.binfmt) can't be set anymore. I assume a patch to remove that
symbol too is also part of that series. Is that correct?


Paul Bolle

Re: RFC arch/sh/kernel/traps_64.c - unclear if/else construct (hpux compat layer)

[Helge Deller]

H Paul,

On 17.02.2015 09:49, Paul Bolle wrote:
> On Mon, 2015-02-16 at 22:24 +0100, Helge Deller wrote:
>> The hpux code is broken anyway.
>> I'm going to remove it from the tree.
>
> That happened in commit 04c161497716 ("parisc: hpux - Drop support for
> HP-UX binaries"), which is included in today's linux-next
> (next-20150217).

Yes.

> That commit removed the Kconfig symbol HPUX. As a result BINFMT_SOM (see
> fs/Kconfig.binfmt) can't be set anymore. I assume a patch to remove that
> symbol too is also part of that series. Is that correct?

I missed it, but just sent out a patch to remove BINFMT_SOM too.
It will be in the next linux-next tree via
http://git.kernel.org/cgit/linux/kernel/git/deller/parisc-linux.git/log/?h=for-next

Helge