Support 32/64 bits build #528
Conversation
zchokri
force-pushed
the
add_support_32bit
branch
from
6cd1c94 to
4805efc
Compare
|
@zchokri - thank you for this pull request. I have a few questions that I will put in the "changes". |
| { | ||
| return (((static_cast<long long int>((ts).seconds_ms) << sizeof((ts).seconds_ls)*8) + | ||
| (ts).seconds_ls)*1000000000LL + (ts).nanoseconds) ; /*!< Converts timestamp value into nanoseconds value*/ | ||
| } | ||
|
|
There was a problem hiding this comment.
Was this change required for building the 32-bit version?
| #if defined(__clang__) && defined(__x86_64__) | ||
| // Clang/llvm has incompatible long double (fp128) for x86_64. | ||
| typedef double FrequencyRatio; /*!< Frequency Ratio */ | ||
| #else |
There was a problem hiding this comment.
This should be a separate commit? It seems un-related to getting 32-bit to compile and run.
| fprintf(stdout, "sync count %u\n", ptpData->sync_count); | ||
| fprintf(stdout, "pdelay count %u\n", ptpData->pdelay_count); | ||
| fprintf(stdout, "asCapable %s\n", ptpData->asCapable ? "True" : "False"); | ||
| fprintf(stdout, "Port State %d\n", (int)ptpData->port_state); | ||
| fprintf(stdout, "process_id %d\n", (int)ptpData->process_id); |
There was a problem hiding this comment.
I think commits in this file should also be a separate commit - I do support these changes.
| static inline u_int32_t TS2NS(struct timespec ts) | ||
| { | ||
| return (((ts).tv_sec*1000000000)+(ts).tv_nsec); | ||
| } |
There was a problem hiding this comment.
Again - was this required for the 32-bit build? Perhaps a separate commit would be better. Also, the (ts).tv_sec could be replaced with ts.tv_sec if you are now using an inline function.
| ifeq ($(32BIT_IOCTL_SUPPORT),y) | ||
| CFLAGS_EXTRA += -DSUPPORT_32BIT_IOCTL | ||
| endif | ||
|
|
There was a problem hiding this comment.
This change seems to me to be the main one relevant to the title of this pull request. I think it should be a separate commit.
There was a problem hiding this comment.
done with commit id : 9c757a4
zchokri
force-pushed
the
add_support_32bit
branch
2 times, most recently
from
9c757a4 to
faff4c2
Compare
This patch enforces the type of returned parameters by changing macro to static inline functions.
zchokri
force-pushed
the
add_support_32bit
branch
from
545ae4c to
a2fb528
Compare
There was a problem hiding this comment.
Hi andrew-elder
Thank you for review.
I split the pull request to 4 commits :
- Change macro to static inline functions a2fb528
- Fix incompatible long double (fp128) for x86_64 75c0b3c
- Print process id and update local time output format 1996135
- Add SUPPORT_32BIT_IOCTL to CCFLAGS to support 32 bit ioctl system call 63a0260
Thanks,
Chokri
| ifeq ($(32BIT_IOCTL_SUPPORT),y) | ||
| CFLAGS_EXTRA += -DSUPPORT_32BIT_IOCTL | ||
| endif | ||
|
|
There was a problem hiding this comment.
done with commit id : 9c757a4
| #if defined(__clang__) && defined(__x86_64__) | ||
| // Clang/llvm has incompatible long double (fp128) for x86_64. | ||
| typedef double FrequencyRatio; /*!< Frequency Ratio */ | ||
| #else |
|
Reviewed on conf call. |
Add SUPPORT_32BIT_IOCTL to CCFLAGS to support 32 bit ioctl system call