Rockbox mail archive

• Next message: George Schaefer: "Player locks up during play"
• Previous message: Chuck D.: "daily build"
• In reply to: Linus Nielsen Feltzing: "Re: id3.c: reading id3 tags, is this a bug?"
• Next in thread: Linus Nielsen Feltzing: "Re: id3.c: reading id3 tags, is this a bug?"
• Reply: Linus Nielsen Feltzing: "Re: id3.c: reading id3 tags, is this a bug?"
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ] [ attachment ]

Examining the latest CVS with Linus Nielsen Feltzing's changes, I'm still not sure that the code is correct. Specifically, if the read fails (bytes read != bytes requested), I wonder why we shouldn't simply bail out. If we aren't bailing out, I suspect that saze should de decreased by bytesread in all cases; currently sometimes it's reduced by bytesread, sometimes by framelen.

One thing especially confuses me:
  static int read_frame(int fd, unsigned char *buf, char **destptr, int framelen)
  {
    int bytesread;
    
    bytesread = read(fd, buf, framelen);
    if(bytesread < 0)
        return bytesread * 10 - 1;

A negative return from read, if I recall correctly, means that the read failed. read_frame's caller is only interested in whether read_frame's return value is less than zero. So why mutiply read's return value by ten, and subtract one? What is this useful for?

Incidently, my original question was motivated by a desire to make some changes to id3.c; rather than anyone else making any changes, I'd be glad to do it.

Thanks.

Quoting Linus Nielsen Feltzing <linus_at_haxx.se>:

> TP Diffenbach wrote:
> > However, size refers to the size of the frame being read from, in
> the
> > mp3's id3 tag. It seems to me that even if unicode_munge is
> adjusting
> > the size of the copy of the string that we are saving, the size
> > remaining on disk in the mp3's id3 tag remains the same. If this
> is
> > the case, size should be set /before/ bytesread is (possibly)
> > adjusted by unicode_munge:
>
> You are absolutely correct. Thanks for pointing that out.
>
> > Also, it seems that the call to read should be checked for error
> or
> > EOF:
>
> I agree.
>
> > Thanks.-- Archos FM needs a Rockbox!
>
> Thank you for finding bugs in our code.
>
> /Linus
>
>

-- 
Archos FM needs a Rockbox!

• Next message: George Schaefer: "Player locks up during play"
• Previous message: Chuck D.: "daily build"
• In reply to: Linus Nielsen Feltzing: "Re: id3.c: reading id3 tags, is this a bug?"
• Next in thread: Linus Nielsen Feltzing: "Re: id3.c: reading id3 tags, is this a bug?"
• Reply: Linus Nielsen Feltzing: "Re: id3.c: reading id3 tags, is this a bug?"
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ] [ attachment ]