On Mon, May 10, 2010 at 12:07:40PM +0200, Kevin Wolf wrote: > > > > - info_begin=read_off(s->fd); > > - if(info_begin==0) > > - goto fail; > > - if(lseek(s->fd,info_begin,SEEK_SET)<0) > > - goto fail; > > We seek to info_begin. > > > - if(read_uint32(s->fd)!=0x100) > > - goto fail; > > Now we are at info_begin + 4 > > > - if((count = read_uint32(s->fd))==0) > > - goto fail; > > info_begin + 8 > > > - info_end = info_begin+count; > > - if(lseek(s->fd,0xf8,SEEK_CUR)<0) > > info_begin + 0x100 > > > + info_begin = read_off(s->fd, offset); > > + if (info_begin == 0) { > > goto fail; > > + } > > + > > + if (read_uint32(s->fd, info_begin) != 0x100) { > > + goto fail; > > + } > > + > > + count = read_uint32(s->fd, info_begin + 4); > > + if (count == 0) { > > + goto fail; > > + } > > + info_end = info_begin + count; > > + > > + offset = info_begin + 0xfc; > > So, wrong offset here?
Yeah, should be 0x100. That's what you get for quickly doing hex calculation in your head. > > + if (type == 0x6d697368 && count >= 244) { > > int new_size, chunk_count; > > - if(lseek(s->fd,200,SEEK_CUR)<0) > > - goto fail; > > + > > + offset += 4; > > Isn't this needed in the else case, too? I don't think so. For that case we previously did a lseek(s->fd,count-4,SEEK_CUR) to undo the 4 byte advance done by the read. > > - s->sectors[i] = last_out_offset+read_off(s->fd); > > - s->sectorcounts[i] = read_off(s->fd); > > - s->offsets[i] = last_in_offset+read_off(s->fd); > > - s->lengths[i] = read_off(s->fd); > > + read_uint32(s->fd, offset); > > This read is useless. offset += 4 alone should be enough. Thanks, fixed. > > /* we need to buffer, because only the chunk as whole can be > > * inflated. */ > > i=0; > > do { > > - ret = read(s->fd, s->compressed_chunk+i, s->lengths[chunk]-i); > > + ret = pread(s->fd, s->compressed_chunk+i, s->lengths[chunk]-i, > > + s->offsets[chunk]); > > This is in a loop, whereas the lseek was outside the loop. From the > second iteration on you'll repeat the first read instead of advancing. You're right. The EINTR check confused me an I took this for just retrying reads on EINTR. Now this code i quite nasty for error returns except EINTR because we'll subtract one from the i loop iteration, yikes. I'll just reuse the i variable to keep the same kind of bug for both sides of the equation. God, do I hate this code..