awslabs / amazon-kinesis-video-streams-pic

Apache License 2.0
49 stars 47 forks source link

Fix definition of LP64 for gcc #214

Closed daveisfera closed 9 months ago

daveisfera commented 1 year ago

Issue #, if available:

213

Description of changes: Define long model correctly for gcc

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

NOTE: Ideally, all of this custom header code that defines types and macros would be removed and the standard headers would be used, but this is at least a step in the right direction

codecov-commenter commented 1 year ago

Codecov Report

Attention: 26 lines in your changes are missing coverage. Please review.

Comparison is base (87f42b6) 75.20% compared to head (8cc367c) 75.15%. Report is 19 commits behind head on develop.

:exclamation: Current head 8cc367c differs from pull request most recent head 4795040. Consider uploading reports for the commit 4795040 to get more accurate results

Files Patch % Lines
src/utils/src/TimerQueue.c 0.00% 14 Missing :warning:
src/utils/src/Threadpool.c 81.48% 5 Missing :warning:
src/client/src/Stream.c 20.00% 4 Missing :warning:
src/client/src/StreamState.c 84.61% 2 Missing :warning:
src/client/src/StreamEvent.c 87.50% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #214 +/- ## =========================================== - Coverage 75.20% 75.15% -0.05% =========================================== Files 52 52 Lines 10182 10225 +43 =========================================== + Hits 7657 7685 +28 - Misses 2525 2540 +15 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

daveisfera commented 1 year ago

I also noticed that the definition of constants used LL so that made the type long long and should only be long on Linux, so using INT64 makes that consistent

hassanctech commented 9 months ago

This fails on MacOS in the CI for both GCC and Clang:

In file included from /Users/runner/work/amazon-kinesis-video-streams-pic/amazon-kinesis-video-streams-pic/src/client/include/com/amazonaws/kinesis/video/client/Include.h:13,
                 from /Users/runner/work/amazon-kinesis-video-streams-pic/amazon-kinesis-video-streams-pic/src/client/src/Include_i.h:16,
                 from /Users/runner/work/amazon-kinesis-video-streams-pic/amazon-kinesis-video-streams-pic/src/client/src/AckParser.c:6:
/Users/runner/work/amazon-kinesis-video-streams-pic/amazon-kinesis-video-streams-pic/src/common/include/com/amazonaws/kinesis/video/common/CommonDefs.h:79: warning: "__LP64__" redefined
   79 | #define __LP64__ // Linux uses LP64 data model
      | 
<built-in>: note: this is the location of the previous definition
In file included from /usr/local/Cellar/gcc@11/11.4.0/lib/gcc/11/gcc/x86_64-apple-darwin21/11/include/stdint.h:9,
                 from /Users/runner/work/amazon-kinesis-video-streams-pic/amazon-kinesis-video-streams-pic/src/common/include/com/amazonaws/kinesis/video/common/CommonDefs.h:118,
                 from /Users/runner/work/amazon-kinesis-video-streams-pic/amazon-kinesis-video-streams-pic/src/client/include/com/amazonaws/kinesis/video/client/Include.h:13,
                 from /Users/runner/work/amazon-kinesis-video-streams-pic/amazon-kinesis-video-streams-pic/src/client/src/Include_i.h:16,
                 from /Users/runner/work/amazon-kinesis-video-streams-pic/amazon-kinesis-video-streams-pic/src/client/src/AckParser.c:6:
/usr/local/Cellar/gcc@11/11.4.0/lib/gcc/11/gcc/x86_64-apple-darwin21/11/include-fixed/stdint.h:18:13: error: #if with no expression
   18 | #if __LP64__
      |             ^