Re: Gnumach cleanup 5 - Control reaches end of non-void function

[Samuel Thibault]

Hi,

Barry deFreese, le Tue 14 Nov 2006 03:59:38 -0500, a écrit :
> I realize that this reverts a couple of the things I sent on my last
> patch but several of the lpr functions really return nothing so should
> have been void in the first place.

Not necessarily.

> Index: i386/i386/trap.c
> ===================================================================
> RCS file: /cvsroot/hurd/gnumach/i386/i386/trap.c,v
> retrieving revision 1.5.2.6
> diff -u -p -r1.5.2.6 trap.c
> --- i386/i386/trap.c  11 Nov 2006 00:54:05 -0000      1.5.2.6
> +++ i386/i386/trap.c  14 Nov 2006 03:43:31 -0000
> @@ -570,6 +570,7 @@ printf("user trap %d error %d sub %08x\n
>  
>       i386_exception(exc, code, subcode);
>       /*NOTREACHED*/
> +     return -1;
>  }

NOTREACHED, hence shouldn't be needed. The right fix is to mark the
declaration of i386_exception with __attribute__((noreturn)).

>  /*
> @@ -1116,6 +1117,8 @@ check_io_fault(regs)
>               /* port not mapped */
>               return FALSE;
>       }
> +     /* NOTREACHED ? */
> +     return FALSE;
>  }

Shouldn't be reached yes, because emulate_io shouldn't return anything
else than what is in the switch statement , but I'd rather put a
default: case in the switch statement.

> -int comprobe(), comintr(), comstart(), commctl();
> -void comattach();
> +int comprobe(), comstart(), commctl();
> +void comattach(), comstop(), comintr();
>  static void comparam();
> -int comstop(), comgetstat(), comsetstat();
> +io_return_t comgetstat(), comsetstat();

If these were defined as such, that was probably for a good reason.  And
the good reason for comstop() for instance is that the t_stop member
of the tty structure is to return an int.  So it must return an int.
Your previous patch just showed the error, don't try to fix it the wrong
way :)

> -int
> +void
>  comintr(unit)

Same here: interrupt handlers are supposed to return an int.

> Index: kern/eventcount.c
> ===================================================================
> RCS file: /cvsroot/hurd/gnumach/kern/eventcount.c,v
> retrieving revision 1.1.1.1.4.6
> diff -u -p -r1.1.1.1.4.6 eventcount.c
> --- kern/eventcount.c 13 Nov 2006 21:30:37 -0000      1.1.1.1.4.6
> +++ kern/eventcount.c 14 Nov 2006 03:43:33 -0000
> @@ -232,6 +232,7 @@ kern_return_t evc_wait_clear(natural_t e
>       simple_unlock(&ev->lock);
>       splx(s);
>       ret = KERN_NO_SPACE; /* XX */
> +     return ret;
>  }

This will probably need more digging than just assuming ret was intended
to be returned, at least just because there is XX here.

> @@ -21,7 +21,7 @@
>   *      Author: Bryan Ford, University of Utah CSL
>   */
>  
> -int putchar(int c)
> +void putchar(int c)
>  {
>       cnputc(c);
>  }

putchar is a gcc built-in, supposed to return an int, so should rather
return an int.

Don't go wild ;)

Samuel