On Thu, 3 Feb 2000, Andreas Beck wrote:

> > +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.

I want to have reading some different file-formats and not only one.
That's why I hack the file-target.

> 
> > +/* 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 ?

You mean, I should use FILE * from 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;

Maybe, when I use the FILE * from the libc, that then bug is fixed.

> 
> This is not only independent of host byte ordering, it will also work.
> 
> > +   for (; strlen > 0; strlen--) {
> More elegant: while(strlen--); 

TNX for that tip.

> 
> > +           _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.

I see. But maybe is file_type, not file_type_op_t, no?


Christoph Egger
E-Mail: [EMAIL PROTECTED]

Reply via email to