> +struct file_type_op_t file_type[] =
> +{
> +        { "ppm", _ggi_file_detect_ppm, NULL, _ggi_file_ppm_write },
> +        { "bmp", _ggi_file_detect_bmp, _ggi_file_bmp_read, NULL },
> +        { NULL, NULL, NULL, NULL }
> +};

Hmm - I'd like it better, if it would read only P?M and use ppmtools to
convert any other format as required.

> +/* Read primitives */
> +
> +void _ggi_file_read_byte(ggi_visual *vis, int *val)
> +{
> +     FileHook *ff = FILE_PRIV(vis);
> +
> +#if 0
> +     if (ff->buf_len > 0) {
> +             (uint8) val = ff->buffer[ff->buf_len++];
> +             return;
> +     }
> +
> +     if ((ff->buf_len = read(LIBGGI_FD(vis), ff->buffer, FILE_BUFFER_SIZE) < 0) {
> +             perror("display-file: read error");
> +     }
> +
> +     if (ff->buf_len < FILE_BUFFER_SIZE) {
> +             /* FIXME: eof is reached */
> +     }
> +#else
> +
> +     if ((ff->buf_len = read(LIBGGI_FD(vis), ff->buffer, 1)) < 0) {
> +             perror("display-file: read error");
> +     }
> +
> +     if (ff->buf_len == 0) {
> +             /* FIXME: eof is reached */
> +     }
> +
> +     (uint8)(*val) = (uint8)(ff->buffer[0]);
> +#endif
> +}

Why don't you just use a file-stream and leave buffering to the libc ?

> +void _ggi_file_read_word(ggi_visual *vis, int *val)
> +{
> +#ifdef GGI_LITTLE_ENDIAN
> +     _ggi_file_read_byte(vis, (int*)(val)   );
> +     _ggi_file_read_byte(vis, (int*)(val+1) );
> +#else
> +     _ggi_file_read_byte(vis, (int*)(val+1) );
> +     _ggi_file_read_byte(vis, (int*)(val)   );
> +#endif
> +}

Uh - please don't do this. There is sick stuff like PDP byte ordering and
the code you have here doesn't do what you want (reading two bytes into 
a single int - right ?) anyway.

A better way would be to use something like

int sum=0
char byte;
read_byte(...,&byte);sum =byte;
read_byte(...,&byte);sum|=byte<<8;

This is not only independent of host byte ordering, it will also work.

> +     for (; strlen > 0; strlen--) {
More elegant: while(strlen--); 

> +             _ggi_file_read_byte(vis, (int*)(str));
This typecast is very ugly BTW. Moreover you will have to increase str, and
it will only work, if the host is little endian.

> +/* FIXME: compiler fails on using this with: "sizeof applied to an incomplete type" 
>*/
> +#define NUM_FILE_TYPES       (sizeof(file_type) / sizeof(struct file_type_op_t))

You need to have a complete prototype for file_type_op_t included before you
can use sizeof.

CU, Andy

-- 
= Andreas Beck                    |  Email :  <[EMAIL PROTECTED]> =

Reply via email to