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 support of posix_memalloc for Linux #648

Merged
merged 1 commit into from
Jan 23, 2016
Merged

Conversation

stweil
Copy link
Contributor

@stweil stweil commented Oct 29, 2015

posix_memalloc is only declared conditionally in stdlib.h,
so add one of the possible definitions to get the declaration.

Signed-off-by: Stefan Weil sw@weilnetz.de

@@ -225,6 +225,8 @@ CHECK_INCLUDE_FILE("unistd.h" HAVE_UNISTD_H)
include(TestLargeFiles)
OPJ_TEST_LARGE_FILES(OPJ_HAVE_LARGEFILES)

set(CMAKE_REQUIRED_DEFINITIONS -D_POSIX_C_SOURCE=200112L)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please set that only for minimal setup (only for posix_memalloc).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a cmake expert. How can I set that definition only for the posix_memalloc test?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use unset when done

@stweil
Copy link
Contributor Author

stweil commented Oct 30, 2015

I updated the patch (and also fixed the commit message).

@stweil
Copy link
Contributor Author

stweil commented Nov 1, 2015

Patch updated because of commit 38ffbbe.

@mayeut
Copy link
Collaborator

mayeut commented Nov 2, 2015

@stweil,
This PR fails to build under linux & macos. c.f. http://my.cdash.org/index.php?project=OPENJPEG&date=2015-11-01

@stweil
Copy link
Contributor Author

stweil commented Nov 3, 2015

@mayeut: It should be fixed now. The failure only occurred when building thirdparty code, too.

@stweil
Copy link
Contributor Author

stweil commented Nov 3, 2015

Next iteration: Windows does not provide unistd.h.

@stweil
Copy link
Contributor Author

stweil commented Dec 31, 2015

Ping? I think this request can be pulled.

posix_memalign is only declared conditionally in stdlib.h,
so add one of the possible definitions to get the declaration.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
@mayeut
Copy link
Collaborator

mayeut commented Jan 6, 2016

@stweil seems that reverting the fix for tiff broke the build. Can you merge the new master branch in to check that all tests are OK ?

@stweil
Copy link
Contributor Author

stweil commented Jan 6, 2016

@mayeut, obviously the rebased patch passes all checks.

@malaterre
Copy link
Collaborator

That's really bad, the official reference is actually missing from http://pubs.opengroup.org/onlinepubs/009695399/functions/posix_memalign.html. Is the Linux implementation a bit too restrictive ?

@malaterre
Copy link
Collaborator

I am ready to merge thie PR until I understand the comment about feeko/ftello. We had a mechanism where _FILE_OFFSET_BITS == 64 was set. Isn't it the case anymore ?

@stweil
Copy link
Contributor Author

stweil commented Jan 9, 2016

fseeko, ftello have the same requirements as posix_memalign to get declared. In the current git master, the compiler complains about missing declarations for those functions. The patch fixes that problem, too.

@malaterre
Copy link
Collaborator

@stweil that is stlighty incorrect. fseeko/ftello are available as soon as you define _FILE_OFFSET_BITS == 64, hence my request for more comment on your side. Does your compiler requires _POSIX_C_SOURCE ?

@stweil
Copy link
Contributor Author

stweil commented Jan 9, 2016

On my 64 bit Debian GNU Linux, _FILE_OFFSET_BITS is not set (in src/lib/openjp2/opj_config_private.h). Therefore I get these compiler messages:

[  1%] Building C object src/lib/openjp2/CMakeFiles/openjp2.dir/openjpeg.c.o
/openjpeg/src/lib/openjp2/openjpeg.c: In function ‘opj_get_data_length_from_file’:
/openjpeg/src/lib/openjp2/openjpeg.c:101:2: warning: implicit declaration of function ‘fseeko’ [-Wimplicit-function-declaration]
  OPJ_FSEEK(p_file, 0, SEEK_END);
  ^
/openjpeg/src/lib/openjp2/openjpeg.c:102:2: warning: implicit declaration of function ‘ftello’ [-Wimplicit-function-declaration]
  file_length = (OPJ_OFF_T)OPJ_FTELL(p_file);
  ^

Extract from the man page of fseeko:

Feature Test Macro Requirements for glibc (see feature_test_macros(7)):

   fseeko(), ftello():
       _FILE_OFFSET_BITS == 64 || _POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600
       (defining the obsolete _LARGEFILE_SOURCE macro also works)

So defining one of _FILE_OFFSET_BITS, _POSIX_C_SOURCE or _XOPEN_SOURCE fixes the compiler warnings. Maybe _FILE_OFFSET_BITS is set on 32 bit Linux. Then the definition of _POSIX_C_SOURCE is not needed for fseeko, but it won't harm.

detonin added a commit that referenced this pull request Jan 23, 2016
Fix support of posix_memalloc for Linux
@detonin detonin merged commit 3767af5 into uclouvain:master Jan 23, 2016
@detonin
Copy link
Contributor

detonin commented Jan 23, 2016

@stweil Thanks. Merged.

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.

4 participants