Avoid implementations in .h files or #including .c files.

[iphydf]

Also, avoid the need for putting _XOPEN_SOURCE in every test file.

---

This change is 

[hugbubby]

Reviewable status: 0 of 1 LGTMs obtained (waiting on @hugbubby)

---

auto_tests/bootstrap_test.c, line 35 at r1 (raw file):

    }

    const TOX_CONNECTION status = tox_self_get_connection_status(tox_udp);

The documentation for this function says it is deprecated. Are these special cases?

---

testing/misc_tools.c, line 47 at r1 (raw file):

#include "../toxcore/tox.h"

void c_sleep(uint32_t x)

Make this function return an int, -2 or some similarly unused number if WIN32, the result of nanosleep() if not.

---

testing/misc_tools.c, line 134 at r1 (raw file):

{
    switch (level) {
        case TOX_LOG_LEVEL_TRACE:

This case is never called.

---

testing/misc_tools.c, line 160 at r1 (raw file):

    }

    uint32_t index = user_data ? *(uint32_t *)user_data : 0;

Isn't this equivalent to index = user_data?

[hugbubby]

Note: I know nothing about CMake or the build process and so I can't really review the changes to those areas.

[iphydf]

Reviewable status: 0 of 1 LGTMs obtained (waiting on @hugbubby)

---

auto_tests/bootstrap_test.c, line 35 at r1 (raw file):

Previously, hugbubby wrote…

The documentation for this function says it is deprecated. Are these special cases?

This

---

testing/misc_tools.c, line 47 at r1 (raw file):

Previously, hugbubby wrote…

Make this function return an int, -2 or some similarly unused number if WIN32, the result of nanosleep() if not.

Why? We never check the return value. Do we need to?

---

testing/misc_tools.c, line 134 at r1 (raw file):

Previously, hugbubby wrote…

This case is never called.

This function is total. It returns a valid result for any input. The fact that callers don't currently produce that input doesn't mean the function should become partial.

---

testing/misc_tools.c, line 160 at r1 (raw file):

Previously, hugbubby wrote…

Isn't this equivalent to index = user_data?

No, because we dereference the pointer only if it's non-null.

[hugbubby]

Reviewable status: 0 of 1 LGTMs obtained (waiting on @hugbubby)

---

auto_tests/bootstrap_test.c, line 35 at r1 (raw file):

Previously, iphydf wrote…

This

Are you saying "this" as in, "You're right, this function should not be used", or "You're right, these are special cases", or did you make a typo

---

testing/misc_tools.c, line 47 at r1 (raw file):

Previously, iphydf wrote…

Why? We never check the return value. Do we need to?

We may. nanosleep() returns -1 and sets errno if it somehow fails or the process receives a signal interrupt. It's better to leave the function caller the discretion if they decide it's important to check.

[iphydf]

Reviewable status: 0 of 1 LGTMs obtained (waiting on @hugbubby)

---

auto_tests/bootstrap_test.c, line 35 at r1 (raw file):

Previously, hugbubby wrote…

Are you saying "this" as in, "You're right, this function should not be used", or "You're right, these are special cases", or did you make a typo

This function should not be used, but that's not part of this PR.

---

testing/misc_tools.c, line 47 at r1 (raw file):

Previously, hugbubby wrote…

We may. nanosleep() returns -1 and sets errno if it somehow fails or the process receives a signal interrupt. It's better to leave the function caller the discretion if they decide it's important to check.

This is testing code, it's easy to add that if needed. For now, I'd rather cater for the API we actually use. Do you feel strongly about this?

[iphydf]

This is also needed for Solaris.

#1015.

[iphydf]

Turns out it's not strictly needed for Solaris.

[hugbubby]

Reviewable status: 0 of 1 LGTMs obtained (waiting on @hugbubby)

---

auto_tests/bootstrap_test.c, line 35 at r1 (raw file):

Previously, iphydf wrote…

This function should not be used, but that's not part of this PR.

I'll make an issue.

---

auto_tests/monolith_test.cc, line 22 at r2 (raw file):

#define DHT_C_INCLUDED

#include "../testing/misc_tools.c"

Missed one.

---

testing/misc_tools.c, line 47 at r1 (raw file):

Previously, iphydf wrote…

This is testing code, it's easy to add that if needed. For now, I'd rather cater for the API we actually use. Do you feel strongly about this?

No, I just feel it wouldn't hurt. I'm not for blocking pull requests that in absolute terms improve the quality of code, and this does, so I won't block it.

[hugbubby]

Sorry about being a pain in the neck about this, just would rather make a comment and have it be dismissed than miss something.

[iphydf]

@hugbubby that's absolutely the right thing to do. We can discuss anything, and we'll do what we agree on.

[iphydf]

Reviewable status: 0 of 1 LGTMs obtained (waiting on @hugbubby)

---

auto_tests/bootstrap_test.c, line 35 at r1 (raw file):

Previously, hugbubby wrote…

I'll make an issue.

Ok.

---

auto_tests/monolith_test.cc, line 22 at r2 (raw file):

Previously, hugbubby wrote…

Missed one.

No, that's intentional. monolith_test includes all the .c files. But monolith_test is going away soon anyway. I'm keeping this line for now.

[hugbubby]

Reviewable status: complete! 1 of 1 LGTMs obtained