RetroArch
RetroArch copied to clipboard
rzipstream_tell: Fix missing tell for non compressed files
The tell call would always return the compressed stream position.
When the zip isn't compressed, isn't that going to return the incorrect position because it will not factor the header and its metadata?
https://github.com/libretro/RetroArch/blob/908667d97ac1ad370ee61665f777cd996e6ba2aa/libretro-common/vfs/vfs_implementation.c#L598-L601
Is this even correct? This appears to be only reachable if mmap fails: https://github.com/libretro/RetroArch/blob/908667d97ac1ad370ee61665f777cd996e6ba2aa/libretro-common/vfs/vfs_implementation.c#L466-L468 (this won't clear the RFILE_HINT_UNBUFFERED flag set here: https://github.com/libretro/RetroArch/blob/908667d97ac1ad370ee61665f777cd996e6ba2aa/libretro-common/vfs/vfs_implementation.c#L321-L325
Well, I did not added a dependency on file stream. It existed already. I just provide a fix for tell. I suppose transparent file stream support was added to support both compressed / non compressed playlists without a burden, which is not a bad idea. Now I did not checked deeper the quality of the support.
Alright, point taken on the already existing filestream dependencies. No problem then. I will remove the prior complaint. Let's just focus on covering the existing issues pointed out by @Cthulhu-throwaway instead.
When the zip isn't compressed, isn't that going to return the incorrect position because it will not factor the header and its metadata?
I've been told this is not a real concern because of the following:
'If the file isn't compressed, then there is no header - so there will be no offset to correct'.
VFS comments though might be worthy of being addressed.
'If the file isn't compressed, then there is no header - so there will be no offset to correct'.
Haven't checked if the library packs archives (ZIPs that are not compressed) as "compressed", but non-compressed archives do absolutely have a header and tree metadata.
>>> from pathlib import Path
>>> with Path("test.zip").open("rb") as f:
... f.read()
...
b'PK\x03\x04\n\x00\x00\x00\x00\x00,{\x0cU\x04\x8df\xbc\x1a\x00\x00\x00\x1a\x00\x00\x00\x08\x00\x00\x00test.txtNON COMPRESSED TEST TEXT\r\nPK\x01\x02?\x00\n\x00\x00\x00\x00\x00,{\x0cU\x04\x8
df\xbc\x1a\x00\x00\x00\x1a\x00\x00\x00\x08\x00$\x00\x00\x00\x00\x00\x00\x00 \x00\x00\x00\x00\x00\x00\x00test.txt\n\x00 \x00\x00\x00\x00\x00\x01\x00\x18\x00\x18;?\xe3x\xae\xd8\x01\xdd\xdd\xf
2\xd5x\xae\xd8\x01\xdd\xdd\xf2\xd5x\xae\xd8\x01PK\x05\x06\x00\x00\x00\x00\x01\x00\x01\x00Z\x00\x00\x00@\x00\x00\x00\x00\x00'
'If the file isn't compressed, then there is no header - so there will be no offset to correct'.
Haven't checked if the library packs archives (ZIPs that are not compressed) as "compressed", but non-compressed archives do absolutely have a header and tree metadata.
>>> from pathlib import Path >>> with Path("test.zip").open("rb") as f: ... f.read() ... b'PK\x03\x04\n\x00\x00\x00\x00\x00,{\x0cU\x04\x8df\xbc\x1a\x00\x00\x00\x1a\x00\x00\x00\x08\x00\x00\x00test.txtNON COMPRESSED TEST TEXT\r\nPK\x01\x02?\x00\n\x00\x00\x00\x00\x00,{\x0cU\x04\x8 df\xbc\x1a\x00\x00\x00\x1a\x00\x00\x00\x08\x00$\x00\x00\x00\x00\x00\x00\x00 \x00\x00\x00\x00\x00\x00\x00test.txt\n\x00 \x00\x00\x00\x00\x00\x01\x00\x18\x00\x18;?\xe3x\xae\xd8\x01\xdd\xdd\xf 2\xd5x\xae\xd8\x01\xdd\xdd\xf2\xd5x\xae\xd8\x01PK\x05\x06\x00\x00\x00\x00\x01\x00\x01\x00Z\x00\x00\x00@\x00\x00\x00\x00\x00'
What it mean by not compressed, is that it's just a bare non archive file (not a RZIP file), then it has no header and the code handle just that - treat a non rzip file opened with rzip stream as a plain file stream.
What it mean by not compressed, is that it's just a bare non archive file (not a RZIP file), then it has no header and the code handle just that - treat a non rzip file opened with rzip stream as a plain file stream.
Should be renamed from is_compressed to is_archive then, imo. Will avoid confusions like these.