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

no member named 'posix_fallocate' in the global namespace #2358

Closed
heartzwh opened this issue Jun 26, 2018 · 11 comments
Closed

no member named 'posix_fallocate' in the global namespace #2358

heartzwh opened this issue Jun 26, 2018 · 11 comments

Comments

@heartzwh
Copy link

hi everyone,

i want to build pcl for android platform.
when i build pcd_grabber.cpp had a issue.
how to fix the error.
thanx.

system and pcl version

  • Operating System and version: Ubuntu 16.04
  • PCL Version: 1.8.1 newest version cc7fe36

[ 22%] Built target pcl_io_ply
[ 22%] Building CXX object io/CMakeFiles/pcl_io.dir/src/pcd_grabber.cpp.o
In file included from /home/sora/Desktop/pcl-for-android-master/build/pcl/io/src/pcd_grabber.cpp:39:
/home/sora/Desktop/pcl-for-android-master/build/pcl/io/include/pcl/io/low_level_io.h:138:16: error:
no member named 'posix_fallocate' in the global namespace
return ::posix_fallocate(fd, 0, len);
~~^

@taketwo
Copy link
Member

taketwo commented Jun 26, 2018

Hi, this is caused by the recently merged #2325 that uses posix_fallocate function. We already had reports that this function is missing from MacOS, so a fallback implementation was added (#2341, #2354). Could you please try if this same fallback works on Android? I.e. change ifdef guards such that on Android the Apple branch is taken.

@de-vri-es that was such a small innocent PR, wasn't it? 😆

@heartzwh
Copy link
Author

@taketwo thanx for you reply. it works, i build complete without other issue.

@taketwo
Copy link
Member

taketwo commented Jun 26, 2018

Great. Please consider sending a PR.

@de-vri-es
Copy link
Contributor

@de-vri-es that was such a small innocent PR, wasn't it?

It seemed that way back then >.< It's POSIX, it should work everywhere :[

I'm not convinced the OS X implementation will actually work correctly on Android. That specific fcntl only shows up in OS X man pages. I would actually expect compile errors on ::fstore_t and F_PREALLOCATE and friends though. It would be good to test writing/reading a PCD on android with the patch to make sure.

Maybe a third fallback that does a seek+write would make sense. The ::posix_fallocate version could be limited to Linux and *BSD. ftruncate is also a possible candidate as fallback. But it creates sparse files when possible (acceptable), and it isn't guaranteed to be able to make files bigger (problem) on all filesystems.

@taketwo
Copy link
Member

taketwo commented Jun 26, 2018

Hm, I found this piece of code, it simply forwards arguments to fallocate, which comes from "fcntl.h" apparently.

@de-vri-es
Copy link
Contributor

de-vri-es commented Jun 26, 2018

Interesting.. I went for posix_fallocate because it's from the POSIX.1-2001 standard while fallocate is Linux specific. Also, posix_fallocate has emulation for filesystems that don't support fallocate directly.

But it seems Android only has the Linux version, not the POSIX version.. Since fallocate can fail on unsupported filesystems (probably FAT and NTFS), it would need to have a fallback to either writing zeroes or seek+write.

@taketwo
Copy link
Member

taketwo commented Jun 26, 2018

I see. I wonder what would be the best way to approach this.

I've never used PCL on Android (and I am actually surprised someone does), so I don't have time/motivation to properly fix this. What if we just add an Android ifdef branch with fallocate() call? This will make sure the thing at least compiles. And then if someone shows up, reports failures and is committed to fix it, we guide him through adding a fallback?

Though to be honest I don't feel very good about this because we'd be leaving a self-inflicted regression in the codebase :(

@de-vri-es
Copy link
Contributor

For best portability we would have to try several things in order and fall back to the next option when one fails.

I think the logical order then would be: one of posix_fallocate/fallocate/OSX fcntl then ftruncate then seek+write with ifdefs to hide compile time unavailable options.

Quite a hassle, but at least there's a single definition now. I'm also willing to put it into a PR if you agree with that direction.

@taketwo
Copy link
Member

taketwo commented Jun 26, 2018

Sounds reasonable. But you know way more than me about these things, so ultimately please follow your own judging.

I'm also willing to put it into a PR if you agree with that direction.

👍

We'll need an Apple and Android person to test that PR. @heartzwh please stay tuned.

@taketwo
Copy link
Member

taketwo commented Jun 28, 2018

I'll reopen this because the issue is not solved upstream.

@de-vri-es
Copy link
Contributor

Initial PR at #2363 =]

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

No branches or pull requests

3 participants