fgetc() returns EOF before end of file.

[Mega Man]

When I open a binary file and try to read 0xFF with fgetc(), I get EOF (= 0xFFFFFFFF) instead.

The problem is in ps2sdk/ee/libc/src/stdio.c:

Code: Select all

int fgetc(FILE *stream)
{
  char c;
  int ret;

  switch(stream->type) {
    case STD_IOBUF_TYPE_GS:
    case STD_IOBUF_TYPE_SIO:
    case STD_IOBUF_TYPE_STDOUTHOST:
      /* cannot read from stdout or stderr. */
      ret = EOF;
      break;
    default:
      ret = ((fread(&c, 1, 1, stream) == 1) ? (int)c : EOF);
  }
  return (ret);
}

"char c;" need to be replaced by "unsigned char c;", because casting to int will extend the sign. All negative values will be wrongly converted. Can someone change it in the svn repository?

[evilo]

I'll check that tonight.

-> same thing also for getc()

did you run the non regression test present in the ps2sdk ?

regards,
Evilo.

[evilo]

Following your remark, I also noticed that fputc could be wrong as well:

Code: Select all

int fputc(int c, FILE *stream)
{
  char ch;

  ch = (char)c;
  return ((fwrite(&ch, 1, 1, stream) == 1) ? 0 : EOF);
}
#endif

should better be :

Code: Select all

int fputc(int c, FILE *stream)
{
  unsigned char ch = (unsigned char) c;

  return ((fwrite(&ch, 1, 1, stream) == 1) ? 0 : EOF);
}
#endif

do you confirm ?

[EDIT] While reviewing stdio.c, I also noticed that EOF is never raised in case the number of byte read (in fread) is smaller that the number of bytes requested... I guess nobody had an issue with that until now :)

[Mega Man]

do you confirm ?

Yes, because the linux programmer's manual says:
fputc() writes the character c, cast to an unsigned char, to stream. I think it is a good idea to be compatible to linux.

I didn't know anything about the testsuite before. Did you mean the test program in "ee/libc/regress". I run it and it is working, only stat() is not implemented. I think it should be possible to implement stat() by using fioGetstat() or fileXioGetStat().

[evilo]

I found the same error in ee/kernel/stdio.c

Code: Select all

#ifdef F_fio_getc
int fioGetc(int fd)
{
	int c;

	fioRead(fd,&c,1);
	return c;
}
#endif

Else, back to stdio.c I also previously added a constant for the USB_MASS device

Code: Select all

#define STD_IOBUF_TYPE_MASS           64

Code: Select all

static struct {
  char * prefix;
  int len;
  int ret;
} __prefix_types[] = {
    { "cdrom0:", 7, STD_IOBUF_TYPE_CDROM },
    { "cdrom:",  6, STD_IOBUF_TYPE_CDROM },
    { "mc0:",    4, STD_IOBUF_TYPE_MC },
    { "mc1:",    4, STD_IOBUF_TYPE_MC },
    { "host0:",  6, STD_IOBUF_TYPE_HOST },
    { "host:",   5, STD_IOBUF_TYPE_HOST },
    { "mass0:",  6, STD_IOBUF_TYPE_MASS },
    { "mass:",   5, STD_IOBUF_TYPE_MASS },
    { 0, 0 }
};

so __stdio_get_fd_type() doesn't return -1 for that device. I know that this is not really required (since it's working the same anyway), but i find it a little cleaner like this :)

and yes I know, I should use "patch" to specify my files diff, but you know, i'm lazy !


any remarks from a "ps2sdk guru" before I commit everything ?

[evilo]

commited !