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