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
