eal/windows: resolve conversion and truncation warnings
Checks
Commit Message
* Initialize const int NS_PER_SEC with an integer literal instead of
double thereby avoiding implicit conversion from double to int.
* Cast the result of the expression assigned to timspec.tv_nsec to long.
Windows builds generate integer truncation warning for this assignment
since the result of the expression was 8 bytes (LONGLONG) but
on Windows targets is 4 bytes. The value produced for the expression
should safely fit in the long.
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
lib/eal/windows/include/rte_os_shim.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Comments
2023-08-02 13:48 (UTC-0700), Tyler Retzlaff:
> * Initialize const int NS_PER_SEC with an integer literal instead of
> double thereby avoiding implicit conversion from double to int.
>
> * Cast the result of the expression assigned to timspec.tv_nsec to long.
Typo: "timespec".
> Windows builds generate integer truncation warning for this assignment
> since the result of the expression was 8 bytes (LONGLONG) but
> on Windows targets is 4 bytes.
Probably "but **tv_nsec** on Windows targets is 4 bytes".
> The value produced for the expression should safely fit in the long.
>
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
> lib/eal/windows/include/rte_os_shim.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
On Thu, Aug 03, 2023 at 01:29:00AM +0300, Dmitry Kozlyuk wrote:
> 2023-08-02 13:48 (UTC-0700), Tyler Retzlaff:
> > * Initialize const int NS_PER_SEC with an integer literal instead of
> > double thereby avoiding implicit conversion from double to int.
> >
> > * Cast the result of the expression assigned to timspec.tv_nsec to long.
>
> Typo: "timespec".
oops
>
> > Windows builds generate integer truncation warning for this assignment
> > since the result of the expression was 8 bytes (LONGLONG) but
> > on Windows targets is 4 bytes.
>
> Probably "but **tv_nsec** on Windows targets is 4 bytes".
thanks i'll update the wording.
one thing that confuses me a little and this change won't break how the
code already works (just makes the cast redundant) is that for mingw
sizeof(long) is being reported as 8 bytes.
this is in spec relative to the C standard but it does leave me somewhat
concerned if struct timespec as defined in the windows headers crosses
an abi boundary.
have you ever noticed this? any thoughts on it?
>
> > The value produced for the expression should safely fit in the long.
> >
> > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > ---
> > lib/eal/windows/include/rte_os_shim.h | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
>
> Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
thanks!
2023-08-02 15:41 (UTC-0700), Tyler Retzlaff:
> one thing that confuses me a little and this change won't break how the
> code already works (just makes the cast redundant) is that for mingw
> sizeof(long) is being reported as 8 bytes.
>
> this is in spec relative to the C standard but it does leave me somewhat
> concerned if struct timespec as defined in the windows headers crosses
> an abi boundary.
MinGW-w64 shows sizeof(long) == 4 in my tests, both native and cross build.
Which MinGW setup reports sizeof(long) == 8 on Windows target?
On Thu, Aug 03, 2023 at 02:44:45AM +0300, Dmitry Kozlyuk wrote:
> 2023-08-02 15:41 (UTC-0700), Tyler Retzlaff:
> > one thing that confuses me a little and this change won't break how the
> > code already works (just makes the cast redundant) is that for mingw
> > sizeof(long) is being reported as 8 bytes.
> >
> > this is in spec relative to the C standard but it does leave me somewhat
> > concerned if struct timespec as defined in the windows headers crosses
> > an abi boundary.
>
> MinGW-w64 shows sizeof(long) == 4 in my tests, both native and cross build.
> Which MinGW setup reports sizeof(long) == 8 on Windows target?
it must have been a dream, i just checked and i get the results you do.
ignore me i'm tired, thanks for checking though.
@@ -87,7 +87,7 @@
static inline int
rte_clock_gettime(clockid_t clock_id, struct timespec *tp)
{
- const int NS_PER_SEC = 1E9;
+ const int NS_PER_SEC = 1000000000;
LARGE_INTEGER pf, pc;
LONGLONG nsec;
@@ -102,7 +102,7 @@
nsec = pc.QuadPart * NS_PER_SEC / pf.QuadPart;
tp->tv_sec = nsec / NS_PER_SEC;
- tp->tv_nsec = nsec - tp->tv_sec * NS_PER_SEC;
+ tp->tv_nsec = (long)(nsec - tp->tv_sec * NS_PER_SEC);
return 0;
default:
return -1;