Re: [PATCH] fix some compiler warnings in gnumach

[Samuel Thibault]

Hello,

Flavio Cruz, on Thu 31 Dec 2015 19:08:19 +0100, wrote:
> @@ -1666,7 +1666,7 @@ device_get_status (void *d, dev_flavor_t flavor, 
> dev_status_t status,
>       return D_INVALID_OPERATION;
>        else
>       {
> -       struct disk_parms *dp = status;
> +       struct disk_parms *dp = (struct disk_parms *) status;

Mmm, this looks so wrong...  I'd rather actually just remove the
V_GETPARMS code and the whole i386/disk.h header actually: AFAIK this
call is never used, and DEV_GET_SIZE/RECORDS should be used instead
anyway.

> diff --git a/i386/i386/phys.c b/i386/i386/phys.c
> index d55bdd9..8681fba 100644
> --- a/i386/i386/phys.c
> +++ b/i386/i386/phys.c
> @@ -77,7 +77,8 @@ pmap_copy_page(
>       vm_offset_t dst)
>  {
>       vm_offset_t src_addr_v, dst_addr_v;
> -     pmap_mapwindow_t *src_map, *dst_map;
> +     pmap_mapwindow_t *src_map = NULL;
> +     pmap_mapwindow_t *dst_map;

For symmetry, better also initialize dst_map. Gcc happens to be
warning only about src_map for some reason, but it will probably warn
about dst_map someday too.

> @@ -114,13 +114,14 @@ int unit = minor(dev);
>  struct bus_device *isai;
>  struct tty *tp;
>  u_short addr;
> -  
> +caddr_t *caddr = (caddr_t *) &addr;
> +
>       if (unit >= NLPR || (isai = lprinfo[unit]) == 0 || isai->alive == 0)
>               return (D_NO_SUCH_DEVICE);
>       tp = &lpr_tty[unit];
>       addr = (u_short) isai->address;
>       tp->t_dev = dev;
> -     tp->t_addr = *(caddr_t *)&addr;
> +     tp->t_addr = *caddr;

The code looks wrong actually, and should be fixed instead of hiding the
warning.  See in lprstart, tp->t_addr is supposed to be the i/o port,
i.e. addr.  I don't know what went through the mind of who wrote
*(caddr_t*), but AIUI, it's completely not needed, and tp->t_addr = addr
is what we actually want.

> diff --git a/kern/slab.c b/kern/slab.c
> index 4f32c8e..99d7c78 100644
> --- a/kern/slab.c
> +++ b/kern/slab.c
> @@ -1480,9 +1480,21 @@ void slab_info(void)
>  #if MACH_KDB
>  #include <ddb/db_output.h>
>  
> - void db_show_slab_info(void)
> +static int
> +db_printf_wrapper(const char* fmt, ...)
>  {
> -    _slab_info(db_printf);
> +    va_list  listp;
> +
> +    va_start(listp, fmt);
> +    db_printf(fmt, listp);

This looks wrong: db_printf doesn't take a listp, it already builds it
for itself.

Samuel