[dpdk-dev] [PATCH v7 3/9] eal: add additional function overrides in windows header files
Dmitry Kozlyuk
dmitry.kozliuk at gmail.com
Wed Feb 5 20:54:52 CET 2020
> +#include <Windows.h>
Nit: better use lowercase to support cross-compilation from OS with
case-sensitive FS. It's not directly relevant for this patch.
> +static inline int
> +eal_get_thread_affinity_mask(pthread_t threadID, unsigned long *cpuset)
> +{
> + /* Workaround for the lack of a GetThreadAffinityMask()
> + *API in Windows
> + */
> + /* obtain previous mask by setting dummy mask */
> + DWORD dwPrevAffinityMask =
> + SetThreadAffinityMask((HANDLE) threadID, 0x1);
> + /* set it back! */
> + SetThreadAffinityMask((HANDLE) threadID, dwPrevAffinityMask);
> + *cpuset = dwPrevAffinityMask;
> + return 0;
> +}
Shouldn't EAL implementation follow DPDK code style? If so, "threadID" and
"dwPrevAffinityMask" violate naming convention, also there's a bogus TAB.
> +static inline int
> +eal_create_thread(void *threadID, void *threadfunc, void *args)
> +{
> + HANDLE hThread;
> + hThread = CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE)threadfunc,
> + args, 0, (LPDWORD)threadID);
> + if (hThread) {
> + SetPriorityClass(GetCurrentProcess(), REALTIME_PRIORITY_CLASS);
> + SetThreadPriority(hThread, THREAD_PRIORITY_TIME_CRITICAL);
> + }
> + return ((hThread != NULL) ? 0 : E_FAIL);
> +}
Duplicates eal_thread_create() from "eal_thread.c", probably
eal_thread_create() should call pthread_create() as in Linux EAL.
> +static inline int
> +pthread_join(pthread_t thread __attribute__((__unused__)),
> + void **value_ptr __attribute__((__unused__)))
> +{
> + return 0;
> +}
If not implemented, should return EINVAL (ENOSUP is not listed as a valid
error code for this function). Even better, implement trivially with
WaitForSingleObject().
> +static inline int
> +asprintf(char **buffer, const char *format, ...)
> +{
> + int size, ret;
> + va_list arg;
> +
> + va_start(arg, format);
> + size = vsnprintf(NULL, 0, format, arg) + 1;
> + va_end(arg);
> +
> + *buffer = malloc(size);
Missing a check for NULL from malloc().
> +
> + va_start(arg, format);
> + ret = vsnprintf(*buffer, size, format, arg);
> + va_end(arg);
> + if (ret != size - 1) {
> + free(*buffer);
> + return -1;
> + }
> + return ret;
> +}
> +static inline int
> +count_cpu(rte_cpuset_t *s)
> +{
> + unsigned int _i;
Why the underscore? It's not a macro, identifiers are function-local.
> +/*
> + * List definitions.
> + */
> +#define LIST_HEAD(name, type) \
> +struct name { \
> + struct type *lh_first; /* first element */ \
> +}
> +
> #define QMD_TRACE_ELEM(elem)
> #define QMD_TRACE_HEAD(head)
> #define TRACEBUF
Probably ouf of scope for this patchset, but DPDK should probably include
the entire FreeBSD <sys/queue.h> so that we won't need to add it
piece-by-piece like so. It's BSD-3-Clause anyway.
--
Dmitry Kozlyuk
More information about the dev
mailing list