Skip to content

posix: Clean up of struct timeval define and related functions #15628

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 8, 2019
Merged

posix: Clean up of struct timeval define and related functions #15628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fa233cc
libc: minimal: Define time_t and suseconds_t
pfalcon Apr 23, 2019
9b79a84
libc: minimal: Introduce sys/_timeval.h
pfalcon Apr 23, 2019
97df338
posix: Move gettimeofday declaration to <sys/time.h>
pfalcon Apr 23, 2019
c2d3c0e
posix: sys/time.h: Add workaround for outdated newlib used by Xtensa
pfalcon Apr 24, 2019
81c772a
CODEOWNERS: Add entry for lib/libc
pfalcon Apr 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CODEOWNERS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,9 +228,9 @@
/include/zephyr.h @andrewboie @andyross
/kernel/ @andrewboie @andyross
/lib/gui/ @vanwinkeljan
/lib/libc/ @nashif
/lib/os/ @andrewboie @andyross
/lib/posix/ @pfalcon
/lib/libc/minimal/source/stdlib/bsearch.c @balajikulkarni
/kernel/device.c @andrewboie @andyross @nashif
/kernel/idle.c @andrewboie @andyross @nashif
/samples/ @nashif
Expand Down
38 changes: 38 additions & 0 deletions include/posix/sys/time.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright (c) 2019 Linaro Limited
*
* SPDX-License-Identifier: Apache-2.0
*/

#ifndef ZEPHYR_INCLUDE_POSIX_SYS_TIME_H_
#define ZEPHYR_INCLUDE_POSIX_SYS_TIME_H_

#ifdef CONFIG_NEWLIB_LIBC
/* Kludge to support outdated newlib version as used in SDK 0.10 for Xtensa */
#include <newlib.h>

#ifdef __NEWLIB__
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add some more comments here about what's going on. I'm guessing this is all because of xtensa's version of newlib being very old.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment 2 lines above of "Kludge to support outdated newlib version as used in SDK 0.10 for Xtensa" not enough?

#include <sys/_timeval.h>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you expecting more things to end up in _timeval.h? Just curious why to introduce a new file instead of just putting the struct here directly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed that at Connect and commit message explains - I'm introduce that file because newlib has that file with that define. It gets included by various other newlib headers. If we have concurrent definition in some other file (like Zephyr POSIX header), we get conflict. We should not play against newlib, but along with it, reusing its definitions. But then the other libc (minimal) should follow the same file structuring and naming, or we'll never get all the different configurations to work consistently.

Now why newlib put that declaration in a separate is a different matter. I can theorize that as mentioned, struct timeval declaration is needed in various headers, e.g. select.h. But nowhere POSIX would say something like "including select.h should have the effect of including sys/time.h". That's why struct timeval is factored out to a separate (libc-private) header, and fine-grained inclusion happens.

#else
#include <sys/types.h>
struct timeval {
time_t tv_sec;
suseconds_t tv_usec;
};
#endif

#else
#include <sys/_timeval.h>
#endif /* CONFIG_NEWLIB_LIBC */

#ifdef __cplusplus
extern "C" {
#endif

int gettimeofday(struct timeval *tv, const void *tz);

#ifdef __cplusplus
}
#endif

#endif /* ZEPHYR_INCLUDE_POSIX_SYS_TIME_H_ */
7 changes: 0 additions & 7 deletions include/posix/time.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,6 @@ struct itimerspec {
};
#endif

struct timeval {
signed int tv_sec;
signed int tv_usec;
};

#include <kernel.h>
#include <errno.h>
#include "posix_types.h"
Expand Down Expand Up @@ -72,8 +67,6 @@ int timer_gettime(timer_t timerid, struct itimerspec *its);
int timer_settime(timer_t timerid, int flags, const struct itimerspec *value,
struct itimerspec *ovalue);

int gettimeofday(struct timeval *tv, const void *tz);

#ifdef __cplusplus
}
#endif
Expand Down
17 changes: 17 additions & 0 deletions lib/libc/minimal/include/sys/_timeval.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/*
* Copyright (c) 2019 Linaro Limited
*
* SPDX-License-Identifier: Apache-2.0
*/

#ifndef ZEPHYR_LIB_LIBC_MINIMAL_INCLUDE_SYS__TIMEVAL_H_
#define ZEPHYR_LIB_LIBC_MINIMAL_INCLUDE_SYS__TIMEVAL_H_

#include <sys/types.h>

struct timeval {
time_t tv_sec;
suseconds_t tv_usec;
};

#endif /* ZEPHYR_LIB_LIBC_MINIMAL_INCLUDE_SYS__TIMEVAL_H_ */
3 changes: 3 additions & 0 deletions lib/libc/minimal/include/sys/types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,7 @@ typedef int off_t;

#endif

typedef long long time_t;
typedef long suseconds_t;

#endif /* ZEPHYR_LIB_LIBC_MINIMAL_INCLUDE_SYS_TYPES_H_ */
1 change: 1 addition & 0 deletions lib/posix/clock.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <kernel.h>
#include <errno.h>
#include <posix/time.h>
#include <posix/sys/time.h>

/*
* `k_uptime_get` returns a timestamp based on an always increasing
Expand Down
1 change: 1 addition & 0 deletions tests/posix/common/src/clock.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/
#include <ztest.h>
#include <posix/time.h>
#include <posix/sys/time.h>
#include <posix/unistd.h>

#define SLEEP_SECONDS 1
Expand Down