Skip to content
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.

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

MSVC support #8

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
7 changes: 6 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,14 @@ set(VIOLET_SOURCES
${CMAKE_CURRENT_SOURCE_DIR}/src/options.c
${CMAKE_CURRENT_SOURCE_DIR}/src/utils.c
)
source_group("Source Files" FILES "${VIOLET_SOURCES}")

set(VIOLET_HEADERS
${CMAKE_CURRENT_SOURCE_DIR}/src/daemon.h
${CMAKE_CURRENT_SOURCE_DIR}/src/options.h
${CMAKE_CURRENT_SOURCE_DIR}/src/utils.h
)
source_group("Header Files" FILES "${VIOLET_HEADERS}")

add_executable(violet ${VIOLET_HEADERS} ${VIOLET_SOURCES})
target_compile_definitions(violet PRIVATE VIOLET_VERSION="${PROJECT_VERSION}")
Expand All @@ -42,7 +44,10 @@ target_link_libraries(violet PRIVATE LibJuice::LibJuiceStatic)

install(TARGETS violet RUNTIME DESTINATION bin)

target_compile_options(violet PRIVATE -Wall -Wextra)
target_compile_options(violet PRIVATE -Wall)
Copy link
Owner

@paullouisageneau paullouisageneau Jul 28, 2021

Choose a reason for hiding this comment

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

Does -Wall work for MSVC? Is it translated by the CMake generator?

Copy link
Author

Choose a reason for hiding this comment

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

Well it didn't error at least

if (NOT MSVC)
target_compile_options(violet PRIVATE -Wextra)
endif()

if(WARNINGS_AS_ERRORS)
target_compile_options(violet PRIVATE -Werror)
SamuelMarks marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ $ git submodule update --init --recursive
```bash
$ cmake -B build
$ cd build
$ make -j2
$ cmake --build .
```

## Running
Expand Down
7 changes: 7 additions & 0 deletions src/daemon.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,14 @@
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>

#ifdef _MSC_VER
//#include <minwindef.h>
typedef unsigned long DWORD;
typedef DWORD pid_t;
#else
#include <unistd.h>
#endif

static void signal_handler(int sig) { (void)sig; }

Expand Down
9 changes: 9 additions & 0 deletions src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,12 @@
#include <stdlib.h>
#include <string.h>
#include <time.h>
#ifdef _MSC_VER
#include <processthreadsapi.h>
#define localtime_r(timer, buf) localtime_s(buf, timer)
Copy link
Owner

Choose a reason for hiding this comment

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

Since the return type is also different, I think defining a proper helper function like in libjuice would be better:

static int get_localtime(const time_t *t, struct tm *buf) {
#ifdef _WIN32
        return localtime_s(buf, t) == 0 ? 0 : -1;
#else // POSIX
        return localtime_r(t, buf) != NULL ? 0 : -1;
#endif
}

#else
#include <unistd.h>
#endif

static FILE *log_file = NULL;

Expand Down Expand Up @@ -85,7 +90,11 @@ int main(int argc, char *argv[]) {
goto error;
}

#ifdef _MSC_VER
SuspendThread(/*TODO*/);
Copy link
Owner

Choose a reason for hiding this comment

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

This is still WIP, right?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah. My main question at the moment is whether running a STUN & TURN server on a node will even make a difference, i.e., if this use-case could be facilitated: https://stackoverflow.com/q/68600741

(my thinking is not which is why I've quasi-abandoned my contribution to the various open-source C STUN/TURN implementations)

Copy link
Owner

Choose a reason for hiding this comment

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

STUN est basically the standard way to implement NAT hole-punching (in particular in the context of ICE). The STUN server must be outside of the NAT you are trying to traverse (hole-punching always relies on a reachable third node outside the NAT), therefore, deploying a STUN server on one of the two connecting nodes won't make any difference.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah that's the same conclusion I came to (eventually).

😢

#else
pause();
#endif

juice_server_destroy(server);

Expand Down
8 changes: 7 additions & 1 deletion src/options.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,19 @@
#include "utils.h"

#include <ctype.h>
#include <getopt.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#ifdef _MSC_VER
#include <BaseTsd.h>
typedef SSIZE_T ssize_t;
#else
#include <getopt.h>
#endif

static char *alloc_string_copy(const char *src, size_t max) {
if (!src)
return NULL;
Expand Down
5 changes: 5 additions & 0 deletions src/utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,12 @@

#include "utils.h"

#ifdef _MSC_VER
#include <string.h>
#define strcasecmp _stricmp
#else
#include <strings.h>
#endif

const char *log_level_to_string(juice_log_level_t level) {
switch (level) {
Expand Down