Fix warnings

[iphydf]

This change is

[tux3]

Review status: 0 of 13 files reviewed at latest revision, 2 unresolved discussions.

---

auto_tests/dht_test.c, line 351 [r1] (raw file):

}

#if 0

Why is this code compiled-out?
If the #if 0 is only meant to be temporary, it would be good to have a comment explaining why this should not be removed but should not be run, so readers 10 years from now will not be left wondering :)
If it's entirely broken though, perhaps it belongs in the git history until a fix is available.

The same remark applies to other commented-out sections below.

---

testing/tox_shell.c, line 83 [r1] (raw file):

    if (ret == -1) {
        printf("fork failed\n");
        free(master);

Are you freeing on return from main to get a clean valgrind output?

---

Comments from Reviewable

[tux3]

Reviewed 13 of 13 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

Comments from Reviewable

[iphydf]

Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

auto_tests/dht_test.c, line 351 [r1] (raw file):

Previously, tux3 (Tux3) wrote…

Why is this code compiled-out?
If the #if 0 is only meant to be temporary, it would be good to have a comment explaining why this should not be removed but should not be run, so readers 10 years from now will not be left wondering :)
If it's entirely broken though, perhaps it belongs in the git history until a fix is available.

The same remark applies to other commented-out sections below.

The `DEFTESTCASE` below was commented out already. This change just comments out the tests themselves as well to avoid confusion (i.e. why are these not called?).

---

testing/tox_shell.c, line 83 [r1] (raw file):

Previously, tux3 (Tux3) wrote…

Are you freeing on return from main to get a clean valgrind output?

Mostly to make the static analyser happy, but yeah, it'll also make valgrind happier :).

---

Comments from Reviewable

[tux3]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

auto_tests/dht_test.c, line 351 [r1] (raw file):

Previously, iphydf wrote…

The DEFTESTCASE below was commented out already. This change just comments out the tests themselves as well to avoid confusion (i.e. why are these not called?).

You're right, they were commented out already. In a spirit of continuous improvement, shouldn't they be removed if no one can explain why they are commented out and not deleted? Git will do what it does and keep a trace if anyone needs to revive dead code.

---

Comments from Reviewable

[iphydf]

Review status: 12 of 13 files reviewed at latest revision, 1 unresolved discussion.

---

auto_tests/dht_test.c, line 351 [r1] (raw file):

Previously, tux3 (Tux3) wrote…

You're right, they were commented out already. In a spirit of continuous improvement, shouldn't they be removed if no one can explain why they are commented out and not deleted?
Git will do what it does and keep a trace if anyone needs to revive dead code.

I've added a comment and commented out the whole unused block. I'd like to keep the non-working tests around, so we can remember to fix them.

---

Comments from Reviewable

[tux3]

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[tux3]

---

Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[nurupo]

What is the purpose of this? This statement has no effect on anything.

[GrayHatter]

this is something that's been in toxav for some time. I had it fixed in a ToxAV branch I was working on, but I abandoned it before it got merged. That's the only reason it still exists, irungentoo and mannol were both aware, but waiting to merge this fix with my AV changes.