Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some crashes #103

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix some crashes #103

wants to merge 2 commits into from

Conversation

pasnox
Copy link

@pasnox pasnox commented Jun 26, 2022

  • Fix memcpy
  • Fix large file support

@anzz1
Copy link
Contributor

anzz1 commented Jun 17, 2023

Not aware of any mame2000 roms which are larger than 2GB, thus confused of the usefulness factor of LFS.

In any case, _FILE_OFFSET_BITS is platform-, compiler- and C-library-specific and should definitely not be added to the common makefile, blanketly applying it to each and every build.

Not all platforms supported by libretro even have largefile support, and on those that do, the 32-bit ones suffer a performance degradation by using it, and for no good reason in this case. For 64-bit ones, using largefile could be warranted. However, in most cases it's fine leaving it undefined and let the compiler decide.

If whatever platform/compiler you were compiling this under had an issue with not defining it, you should add the define in the platform-specific Makefile , not the common one shared by every platform.

@pasnox
Copy link
Author

pasnox commented Jun 18, 2023

@anzz1 The LFS addition was not really added to get support for files larger than 2GB. I'm using rclone (See rclone.org) to mount remote / cloud data locally. And accessing those mount points require LFS be enabled. else the readdir handles returns errors (can't remember exactly details by now, but it was some sort of overflow in the returned handles). As if the FUSE based mount was enabled only with LFS and not accessible without. It could be very well, that rclone mounts should not enforce it if not required, but I did not get any feedback on that mater from the rclone project. I'm not familiar with the fs drivers, support etc. If you have more information that would be helpful.

@anzz1
Copy link
Contributor

anzz1 commented Jun 19, 2023

@anzz1 The LFS addition was not really added to get support for files larger than 2GB. I'm using rclone (See rclone.org) to mount remote / cloud data locally. And accessing those mount points require LFS be enabled. else the readdir handles returns errors (can't remember exactly details by now, but it was some sort of overflow in the returned handles). As if the FUSE based mount was enabled only with LFS and not accessible without. It could be very well, that rclone mounts should not enforce it if not required, but I did not get any feedback on that mater from the rclone project. I'm not familiar with the fs drivers, support etc. If you have more information that would be helpful.

That would be a special case then, better added as a patch to your own Makefile for your usecase, not this project. Like said, LFS isn't even supported by every platform and not needed in typical cases when it is, so it can't be blanketly added to the common Makefile like that.

I did actually look into rclone a while ago, but ultimately didn't bother with it since there was all these dumb obstacles in setting it up, making it unnecessarily frustrating to make a simple user-facing configuration interface to set it up in cases like embedded devices which do not have a web browser. This isn't a fault of the rclone project though, but the cloud services it's using, as they have deprecated the simpler APIs and now require more complicated steps including using a PC and a web browser to be able to login.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants