On Thursday 20 October 2011 01:38:46 Che-Liang Chiou wrote:
> --- a/api/api.c
> +++ b/api/api.c
>
> +/*
> + * pseudo signature:
> + *
> + * int API_display_get_info(int type, struct display_info *di)
> + */
> +static int API_display_get_info(va_list ap)
> +{
> + int type;
> + struct display_info *di;
> +
> + type = (int)va_arg(ap, u_int32_t);
> + di = (struct display_info *)va_arg(ap, u_int32_t);
i know you simply copied these warts from the rest of the code, but this
va_arg() usage looks wrong to me. why is u_int32_t always used as the
argument to va_arg() and then the return value is cast ? seems to me this
code should actually read:
type = va_arg(ap, int);
di = va_arg(ap, struct display_info *);
could you see if that change still works for you ?
> +static int API_display_draw_bitmap(va_list ap)
> +{
> + ulong bitmap;
> + int x, y;
> +
> + bitmap = (ulong)va_arg(ap, ulong);
> + if (!bitmap)
> + return API_EINVAL;
seems to me that this NULL pointer check belongs in the lcd core
> --- /dev/null
> +++ b/api/api_display.c
>
> +int display_draw_bitmap(ulong bitmap, int x, int y)
> +{
> + int err = 0;
> +#ifdef CONFIG_LCD
> + err |= lcd_display_bitmap(bitmap, x, y);
> +#endif
> + return err;
> +}
i think this should read:
int display_draw_bitmap(ulong bitmap, int x, int y)
{
#ifdef CONFIG_LCD
return lcd_display_bitmap(bitmap, x, y);
#else
return API_ENODEV;
#endif
}
> --- a/include/video_font.h
> +++ b/include/video_font.h
>
> +/*
> + * Adding unused attribute here so that compiler will not complain
> + * video_fontdata is not used in source files that only use
> + * VIDEO_FONT_* (e.g., api/api_display.c).
> + */
> +__attribute__ ((unused))
> static unsigned char video_fontdata[VIDEO_FONT_SIZE] = {
i don't think this is right. we should split the data from the rest of the
defines/structs.
-mike
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
