inclusion of WinFlex changes (drop need for posix functions fork/wait, htonl/htons) #367
Comments
GitMensch
changed the title
|
note: the Opinions? Note: I actually try to build flex with OrangeC with the goal to use flex' testsuite as an additional test bed for this compiler (compilation works [when cheating an empty sys/wait.h] after their latest changes, linking stops on the non-posix parts that seem to be replaced in the flex parts of WinFlexBison, which ideally should have PR'd their changes in the first place... |
|
Note: if this is agreed upon I'd add some PR with getting similar changes in that are needed to compile and run with OrangeC and VS (as it seems to be the main target of the WinFlexBison changes). |
|
@GitMensch I have discovered the non-portability of htonl / htons, but I'm not sure how WinFlexBison solves it. (htonl and htons are used in skeleton, so adding a check of them in configure is useless. We need other workaround for the case.) Can you give a better pointer? Regarding the others, I can't comment at the moment. |
We likely should split the issue into multiple PR. I assume the @Explorer09 What do you mean by "used in the skeleton"? Isn't it only used in src/tables.c? Obviously
It doesn't use the functions but defines instead. This would be a first step in any case. Then define it depending on #define SWAP_BYTE_ORDER_UINT(val) (((val << 24) & 0xFF000000) |\
((val << 8) & 0x00FF0000) |\
((val >> 8) & 0x0000FF00) |\
((val >> 24) & 0x000000FF) )
#define SWAP_BYTE_ORDER_USHORT(val) (((val << 8) & 0xFF00) |\
((val >> 8) & 0x00FF) )byte swapping functions are included in many headers, for example libcob/common.h: /* Byte swap functions */
/*
The original idea for the byteswap routines was taken from GLib.
(Specifically glib/gtypes.h)
GLib is licensed under the GNU Lesser General Public License.
*/
/* Generic swapping functions */
#undef COB_BSWAP_16_CONSTANT
#undef COB_BSWAP_32_CONSTANT
#undef COB_BSWAP_64_CONSTANT
#undef COB_BSWAP_16
#undef COB_BSWAP_32
#undef COB_BSWAP_64
#define COB_BSWAP_16_CONSTANT(val) ((cob_u16_t) ( \
(((cob_u16_t)(val) & (cob_u16_t) 0x00FFU) << 8) | \
(((cob_u16_t)(val) & (cob_u16_t) 0xFF00U) >> 8)))
#define COB_BSWAP_32_CONSTANT(val) ((cob_u32_t) ( \
(((cob_u32_t) (val) & (cob_u32_t) 0x000000FFU) << 24) | \
(((cob_u32_t) (val) & (cob_u32_t) 0x0000FF00U) << 8) | \
(((cob_u32_t) (val) & (cob_u32_t) 0x00FF0000U) >> 8) | \
(((cob_u32_t) (val) & (cob_u32_t) 0xFF000000U) >> 24)))
#define COB_BSWAP_64_CONSTANT(val) ((cob_u64_t) ( \
(((cob_u64_t) (val) & \
(cob_u64_t) COB_U64_C(0x00000000000000FF)) << 56) | \
(((cob_u64_t) (val) & \
(cob_u64_t) COB_U64_C(0x000000000000FF00)) << 40) | \
(((cob_u64_t) (val) & \
(cob_u64_t) COB_U64_C(0x0000000000FF0000)) << 24) | \
(((cob_u64_t) (val) & \
(cob_u64_t) COB_U64_C(0x00000000FF000000)) << 8) | \
(((cob_u64_t) (val) & \
(cob_u64_t) COB_U64_C(0x000000FF00000000)) >> 8) | \
(((cob_u64_t) (val) & \
(cob_u64_t) COB_U64_C(0x0000FF0000000000)) >> 24) | \
(((cob_u64_t) (val) & \
(cob_u64_t) COB_U64_C(0x00FF000000000000)) >> 40) | \
(((cob_u64_t) (val) & \
(cob_u64_t) COB_U64_C(0xFF00000000000000)) >> 56)))
/* Machine/OS specific overrides */
#ifdef __GNUC__
#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 3)
#define COB_BSWAP_16(val) (COB_BSWAP_16_CONSTANT (val))
#define COB_BSWAP_32(val) (__builtin_bswap32 (val))
#define COB_BSWAP_64(val) (__builtin_bswap64 (val))
#elif defined(__i386__)
#define COB_BSWAP_16(val) (COB_BSWAP_16_CONSTANT (val))
#define COB_BSWAP_32(val) \
(__extension__ \
({ register cob_u32_t __v, \
__x = ((cob_u32_t) (val)); \
if (__builtin_constant_p (__x)) \
__v = COB_BSWAP_32_CONSTANT (__x); \
else \
__asm__ ("bswap %0" \
: "=r" (__v) \
: "0" (__x)); \
__v; }))
#define COB_BSWAP_64(val) \
(__extension__ \
({ union { cob_u64_t __ll; \
cob_u32_t __l[2]; } __w, __r; \
__w.__ll = ((cob_u64_t) (val)); \
if (__builtin_constant_p (__w.__ll)) \
__r.__ll = COB_BSWAP_64_CONSTANT (__w.__ll); \
else \
{ \
__r.__l[0] = COB_BSWAP_32 (__w.__l[1]); \
__r.__l[1] = COB_BSWAP_32 (__w.__l[0]); \
} \
__r.__ll; }))
#elif defined (__ia64__)
#define COB_BSWAP_16(val) (COB_BSWAP_16_CONSTANT (val))
#define COB_BSWAP_32(val) \
(__extension__ \
({ register cob_u32_t __v, \
__x = ((cob_u32_t) (val)); \
if (__builtin_constant_p (__x)) \
__v = COB_BSWAP_32_CONSTANT (__x); \
else \
__asm__ __volatile__ ("shl %0 = %1, 32 ;;" \
"mux1 %0 = %0, @rev ;;" \
: "=r" (__v) \
: "r" (__x)); \
__v; }))
#define COB_BSWAP_64(val) \
(__extension__ \
({ register cob_u64_t __v, \
__x = ((cob_u64_t) (val)); \
if (__builtin_constant_p (__x)) \
__v = COB_BSWAP_64_CONSTANT (__x); \
else \
__asm__ __volatile__ ("mux1 %0 = %1, @rev ;;" \
: "=r" (__v) \
: "r" (__x)); \
__v; }))
#elif defined (__x86_64__)
#define COB_BSWAP_16(val) (COB_BSWAP_16_CONSTANT (val))
#define COB_BSWAP_32(val) \
(__extension__ \
({ register cob_u32_t __v, \
__x = ((cob_u32_t) (val)); \
if (__builtin_constant_p (__x)) \
__v = COB_BSWAP_32_CONSTANT (__x); \
else \
__asm__ ("bswapl %0" \
: "=r" (__v) \
: "0" (__x)); \
__v; }))
#define COB_BSWAP_64(val) \
(__extension__ \
({ register cob_u64_t __v, \
__x = ((cob_u64_t) (val)); \
if (__builtin_constant_p (__x)) \
__v = COB_BSWAP_64_CONSTANT (__x); \
else \
__asm__ ("bswapq %0" \
: "=r" (__v) \
: "0" (__x)); \
__v; }))
#else /* Generic gcc */
#define COB_BSWAP_16(val) (COB_BSWAP_16_CONSTANT (val))
#define COB_BSWAP_32(val) (COB_BSWAP_32_CONSTANT (val))
#define COB_BSWAP_64(val) (COB_BSWAP_64_CONSTANT (val))
#endif
#elif defined(_MSC_VER)
#define COB_BSWAP_16(val) (_byteswap_ushort (val))
#define COB_BSWAP_32(val) (_byteswap_ulong (val))
#define COB_BSWAP_64(val) (_byteswap_uint64 (val))
#else /* Generic */
#define COB_BSWAP_16(val) (COB_BSWAP_16_CONSTANT (val))
#define COB_BSWAP_32(val) (COB_BSWAP_32_CONSTANT (val))
#define COB_BSWAP_64(val) (COB_BSWAP_64_CONSTANT (val))
#endif
/* End byte swap functions */ |
|
@GitMensch I believe making htons/htonl macro defines would cause more portability problem than it solves, since (I think) some compilers would recognize htons/htonl functions and optimize them into inline assembly without generating function calls. Repeating what a compiler would do in a header is a bad idea. I think with portability to Windows, including <winsock.h> would be the way to go. But for even more exotic platforms, I don't know if we would need this naïve fallback: unsigned short htons(unsigned short val) {
unsigned char arr[sizeof(short)] = {0};
int i;
for (i = sizeof(short) - 1; i >= 0; i--) {
arr[i] = (unsigned char)(val);
val >>= CHAR_BIT;
}
return (*(unsigned short *)arr);
}(Yes, you get it. This is the most portable htons implementation within ANSI C constraints.) |
|
If we're going to provide an implementation of a function, we should use the libobj mechanism.
…On Sunday, 24 June 2018, 3:39 am -0700, "Kang-Che Sung (宋岡哲)" ***@***.***> wrote:
@GitMensch
Maybe I was wrong about htons/htonl pair, but their inverse, `ntohs` and `ntohl`, have to be included in the skeleton as it is part of the decoder of the flex table file format.
I believe making htons/htonl macro defines would cause more portability problem than it solves, since (I think) some compilers would recognize htons/htonl functions and optimize them into inline assembly without generating function calls. Repeating what a compiler would do in a header is a bad idea.
I think with portability to Windows, including <winsock.h> would be the way to go. But for even more exotic platforms, I don't know if we would need this naive fallback:
```c
short htons(short val) {
unsigned char arr[sizeof(short)] = {0};
int i;
for (i = sizeof(short) - 1; i >= 0; i--) {
arr[i] = (unsigned char)(val);
val >>= CHAR_BIT;
}
return (*(short *)arr);
}
```
(Yes, you get it. This is the most portable htons implementation within ANSI C constraints.)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
#367 (comment)
--
Will Estes
westes575@gmail.com
|
|
@westes I think the licensing problem (it's LGPL) already prevents us from including libcob's header directly info flex's source. |
|
libcob?
…On Sunday, 24 June 2018, 6:29 am -0700, "Kang-Che Sung (宋岡哲)" ***@***.***> wrote:
@westes I think the licensing problem (it's LGPL) already prevents us from including libcob's header directly info flex's source.
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#367 (comment)
--
Will Estes
westes575@gmail.com
|
|
@westes I was talking about libcob that @GitMensch mentioned. And I had no idea what is "libobj mechanism" you are talking about. |
|
libobj as handled by automake. It's the way to provide portability functions when absent from the system at compile time.
…On Sunday, 24 June 2018, 6:36 am -0700, "Kang-Che Sung (宋岡哲)" ***@***.***> wrote:
@westes I was talking about libcob that @GitMensch mentioned. And I had no idea what is "libobj mechanism" you are talking about.
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#367 (comment)
--
Will Estes
westes575@gmail.com
|
|
@westes |
|
Hello everyone, when I ported flex to Windows I used htons/htonl functions from <winsock.h>. I can easily revert back my "improvement" and use htons/htonl again. ntohs/ntohl are used in generated code only. I didn't changed this code so parsers are dependent from Ws2_32.dll under windows now. And I think it's not a big deal. In summary, I don't think ntohs/ntohl is a problem at all. |
|
@lexxmark Please revert this change to use functions from winsock.h. I'll then try to create a PR to this repo which includes the checks for This should be a good start to minimize the differences between this repo and your port. |
|
@lexxmark If you read my post above this issue report, I have presented a code that does endian-independant appraroch to byte swapping (or see this article on the web). If we are to add a htons or htonl in Flex, it would be that most portable version. Otherwise, I would better tell compilers to fix their optimization rather than implement a workaround that would always be inferior to a machine instruction. |
This comment has been minimized.
This comment has been minimized.
|
I've reverted htons/htonl (see my commit) |
copy from lexxmark/winflexbison@2a7586a by @yjh-styx - partial work on westes#367
https://github.com/lexxmark/winflexbison/tree/master/flex/src includes some changes for non-GCC (obviously especially WIN32) compilers.
What do you think about integrating them (after cleanup)?
A good starting example seems to be filter.c (and flexdef.h) to use a different signature of
struct filterandfilter_apply_chain (). These changes could be integrated if one or both functionsfork()/wait()are not available (at least with 2.6.4 there is a check in configure for them but neither a configure time failure nor a#ifdef HAVE_FORKin the code).Similar could be done for tables.c and
htonl/htons(which seem to have no check during configure).I know this would mean to inspect all the changes and merge them one by one, but at least this would be a start for a more common code base.
So... was a discussion about this before?
The text was updated successfully, but these errors were encountered: