search

xiph / ogg

Reference implementation of the Ogg media container
BSD 3-Clause "New" or "Revised" License
345 stars 168 forks source link

missing ogg_uint64_t in configuration generated by cmake #45

Closed malytomas closed 5 years ago

malytomas commented 5 years ago 
include/ogg/config_types.h:24:10: error: ‘ogg_uint64_t’ does not name a type; did you mean ‘ogg_int64_t’?
 typedef  ogg_uint64_t;
          ^~~~~~~~~~~~
          ogg_int64_t

I am getting this error on linux, gcc with cmake generated project. Has the unsigned type been removed?

GekkieHenkie commented 5 years ago

I get the same error on Ubuntu with Clang, also a CMake generated project

include/ogg/config_types.h:24:10: error: C++ requires a type specifier for all declarations:
typedef  ogg_uint64_t;
willson-chen commented 5 years ago

with ubuntu 18.04, gcc 7.4, cmake 3.10.2, I got a warning. include/ogg/config_types.h:24:10: warning: type defaults to 'int' in declaration of 'ogg_uint64_t' [-Wimplicit-int] typedef ogg_uint64_t

I fix the warning by rewrite line 24 in config_types.h. typedef uint64_t ogg_uint64_t And it works.

erikd commented 5 years ago

This is something that is likely fixed in PR #48 which has just been merged.

willson-chen commented 5 years ago

This is something that is likely fixed in PR #48 which has just been merged.

I tried the new commit. But it doesn't work in my environment

erikd commented 5 years ago

@willson-chen You need to provide exact instructions on how you get to that failure so that others can reproduce it.

willson-chen commented 5 years ago

@willson-chen You need to provide exact instructions on how you get to that failure so that others can reproduce it.

I just type these commands simply.

cmake .
make

I tried the following environment.

ubuntu 16.04.02 LTS, x86_64, gcc v5.4, cmake v3.5.1
ubuntu 18.04.02 LTS, x86_64, gcc v7.4, cmake v3.10.2
CentOS 7.2.1511, x86_64, gcc v4.8.5, cmake v2.8.12.2
SUSE 12.2, x86_64, gcc v4.8.5, cmake 3.14.6

I got warning int Ubuntu 16 and Ubuntu 18, no warning in CentOS and SUSE. But when I check the line 24 in include/ogg/config_types.h for CentOS and SUSE, I found it is still typedef ogg_uint64_t. So I think the reason why no warning is that the gcc version is too low.

I don't know if the above instructions is enough? @erikd

malytomas commented 5 years ago

https://github.com/xiph/ogg/blob/688208e6960595b2f33a6dc362033fbe369adc0d/include/ogg/config_types.h.in#L24 There is a macro USIZE64 which is not populated in the original code nor in the #48.

Taking the PR as an inspiration, something like this should be added to cmake/CheckSizes.cmake:

check_type_size("uint64_t" UINT64_SIZE LANGUAGE C)
check_type_size("u_int64_t" U_INT64_SIZE LANGUAGE C)

if(UINT64_SIZE EQUAL 8)
    set(USIZE64 "uint64_t")
elseif(INT_SIZE EQUAL 8)
    set(USIZE64 "unsigned int")
elseif(LONG_SIZE EQUAL 8)
    set(USIZE64 "unsigned long")
elseif(LONG_LONG_SIZE EQUAL 8)
    set(USIZE64 "unsigned long long")
elseif(U_INT64_SIZE EQUAL 8)
    set(USIZE64 "u_int64_t")
else()
    message(FATAL_ERROR "No unsigned 64 bit type found on this platform!")
endif()

Note, I am currently unable to test the code.

erikd commented 5 years ago

@willson-chen Please try the fix in https://github.com/xiph/ogg/pull/52 HEAD.

willson-chen commented 5 years ago

It works this time. @erikd, you are a great maintainer!

erikd commented 5 years ago

Thanks!