<div dir="ltr">Hi David,<div><br></div><div>I'm back home in front of a real keyboard, and want to follow up with you about your patches. I must admit that my usual practice is to use NULL, and not 0. Around the time I wrote altstack I had recently read Jen Gustedt's book, Modern C. In it, he makes the point that NULL hides more than it clarifies (see section 11.7). While I find his argument a little weak when considering a gcc environment where I know that NULL is defined as ((void *)0), it seems sound when the target tool chain isn't known in advance, as with a CCAN module. Here's a link to the book and also a blog article from Jens about NULL. I'm very curious to know if you (or any other CCAN contributors) find his arguments at all compelling. Thanks. -Dan</div><div><br></div><div><a href="http://icube-icps.unistra.fr/img_auth.php/d/db/ModernC.pdf">http://icube-icps.unistra.fr/img_auth.php/d/db/ModernC.pdf</a><br></div><div><a href="https://gustedt.wordpress.com/2010/11/07/dont-use-null/">https://gustedt.wordpress.com/2010/11/07/dont-use-null/</a><br></div><div><br></div><div><div class="gmail_quote"><div dir="ltr">On Fri, Jun 3, 2016 at 4:40 AM David Gibson <<a href="mailto:david@gibson.dropbear.id.au">david@gibson.dropbear.id.au</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">In a number of places the altstack module uses a literal '0' for pointer<br>
values. That's correct C, but doesn't make it obvious on a quick read<br>
whether values are integers or pointers. This patch changes those cases<br>
to use the NULL define instead.<br>
<br>
Signed-off-by: David Gibson <<a href="mailto:david@gibson.dropbear.id.au" target="_blank">david@gibson.dropbear.id.au</a>><br>
---<br>
ccan/altstack/_info | 4 ++--<br>
ccan/altstack/altstack.c | 10 +++++-----<br>
ccan/altstack/altstack.h | 6 +++---<br>
ccan/altstack/test/run.c | 22 +++++++++++-----------<br>
4 files changed, 21 insertions(+), 21 deletions(-)<br>
<br>
diff --git a/ccan/altstack/_info b/ccan/altstack/_info<br>
index 7accf86..b8cf6a8 100644<br>
--- a/ccan/altstack/_info<br>
+++ b/ccan/altstack/_info<br>
@@ -63,8 +63,8 @@<br>
* void *out;<br>
*<br>
* assert(argc == 3);<br>
- * stk_max = strtol(argv[1], 0, 0) * 1024 * 1024;<br>
- * vla_sz = strtol(argv[2], 0, 0) * 1024 * 1024;<br>
+ * stk_max = strtol(argv[1], NULL, 0) * 1024 * 1024;<br>
+ * vla_sz = strtol(argv[2], NULL, 0) * 1024 * 1024;<br>
* assert(stk_max > 0 && vla_sz > 0);<br>
*<br>
* snprintf(maps, sizeof(maps), "egrep '\\[stack' /proc/%d/maps", getpid());<br>
diff --git a/ccan/altstack/altstack.c b/ccan/altstack/altstack.c<br>
index 6dfb9fa..2115740 100644<br>
--- a/ccan/altstack/altstack.c<br>
+++ b/ccan/altstack/altstack.c<br>
@@ -78,10 +78,10 @@ int altstack(rlim_t max, void *(*fn)(void *), void *arg, void **out)<br>
<br>
state.fn = fn;<br>
state.arg = arg;<br>
- state.out = 0;<br>
+ state.out = NULL;<br>
state.max = max;<br>
state.ebuf[state.elen = 0] = '\0';<br>
- if (out) *out = 0;<br>
+ if (out) *out = NULL;<br>
<br>
// if the first page below the mapping is in use, we get max-pgsz usable bytes<br>
// add pgsz to max to guarantee at least max usable bytes<br>
@@ -91,7 +91,7 @@ int altstack(rlim_t max, void *(*fn)(void *), void *arg, void **out)<br>
ok(setrlimit(RLIMIT_STACK, &(struct rlimit) { state.max, rl_save.rlim_max }), 1);<br>
undo++;<br>
<br>
- ok(m = mmap(0, max, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_GROWSDOWN|MAP_NORESERVE, -1, 0), 1);<br>
+ ok(m = mmap(NULL, max, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_GROWSDOWN|MAP_NORESERVE, -1, 0), 1);<br>
undo++;<br>
<br>
if (setjmp(state.jmp) == 0) {<br>
@@ -130,9 +130,9 @@ out:<br>
<br>
switch (undo) {<br>
case 4:<br>
- ok(sigaction(SIGSEGV, &sa_save, 0), 0);<br>
+ ok(sigaction(SIGSEGV, &sa_save, NULL), 0);<br>
case 3:<br>
- ok(sigaltstack(&ss_save, 0), 0);<br>
+ ok(sigaltstack(&ss_save, NULL), 0);<br>
case 2:<br>
ok(munmap(m, max), 0);<br>
case 1:<br>
diff --git a/ccan/altstack/altstack.h b/ccan/altstack/altstack.h<br>
index 4445a2a..09ffe0b 100644<br>
--- a/ccan/altstack/altstack.h<br>
+++ b/ccan/altstack/altstack.h<br>
@@ -52,7 +52,7 @@<br>
* static void *wrap(void *i)<br>
* {<br>
* dn((unsigned long) i);<br>
- * return 0;<br>
+ * return NULL;<br>
* }<br>
*<br>
* #define MiB (1024UL*1024UL)<br>
@@ -60,9 +60,9 @@<br>
* {<br>
* unsigned long n;<br>
* assert(argc == 2);<br>
- * n = strtoul(argv[1], 0, 0);<br>
+ * n = strtoul(argv[1], NULL, 0);<br>
*<br>
- * if (altstack(32*MiB, wrap, (void *) n, 0) != 0)<br>
+ * if (altstack(32*MiB, wrap, (void *) n, NULL) != 0)<br>
* altstack_perror();<br>
*<br>
* printf("%d\n", depth);<br>
diff --git a/ccan/altstack/test/run.c b/ccan/altstack/test/run.c<br>
index 61710fd..e109ccb 100644<br>
--- a/ccan/altstack/test/run.c<br>
+++ b/ccan/altstack/test/run.c<br>
@@ -87,43 +87,43 @@ int main(void)<br>
<br>
plan_tests(50);<br>
<br>
- chkfail(getrlimit_, altstack(8*MiB, wrap, int2ptr(0), 0) == -1, e(getrlimit_),<br>
+ chkfail(getrlimit_, altstack(8*MiB, wrap, int2ptr(0), NULL) == -1, e(getrlimit_),<br>
0,<br>
0);<br>
<br>
- chkfail(setrlimit_, altstack(8*MiB, wrap, int2ptr(0), 0) == -1, e(setrlimit_),<br>
+ chkfail(setrlimit_, altstack(8*MiB, wrap, int2ptr(0), NULL) == -1, e(setrlimit_),<br>
getrlimit_,<br>
0);<br>
<br>
- chkfail(mmap_, altstack(8*MiB, wrap, int2ptr(0), 0) == -1, e(mmap_),<br>
+ chkfail(mmap_, altstack(8*MiB, wrap, int2ptr(0), NULL) == -1, e(mmap_),<br>
getrlimit_|setrlimit_,<br>
setrlimit_);<br>
<br>
- chkfail(sigaltstack_, altstack(8*MiB, wrap, int2ptr(0), 0) == -1, e(sigaltstack_),<br>
+ chkfail(sigaltstack_, altstack(8*MiB, wrap, int2ptr(0), NULL) == -1, e(sigaltstack_),<br>
getrlimit_|setrlimit_|mmap_,<br>
setrlimit_|munmap_);<br>
<br>
- chkfail(sigaction_, altstack(8*MiB, wrap, int2ptr(0), 0) == -1, e(sigaction_),<br>
+ chkfail(sigaction_, altstack(8*MiB, wrap, int2ptr(0), NULL) == -1, e(sigaction_),<br>
getrlimit_|setrlimit_|mmap_|sigaltstack_,<br>
setrlimit_|munmap_|sigaltstack_);<br>
<br>
- chkfail(munmap_, altstack(8*MiB, wrap, int2ptr(0), 0) == 1, e(munmap_),<br>
+ chkfail(munmap_, altstack(8*MiB, wrap, int2ptr(0), NULL) == 1, e(munmap_),<br>
getrlimit_|setrlimit_|mmap_|sigaltstack_|sigaction_,<br>
setrlimit_|sigaltstack_|sigaction_);<br>
if (fail = 0, munmap(m_, msz_) == -1)<br>
err(1, "munmap");<br>
<br>
- chkok( altstack(1*MiB, wrap, int2ptr(1000000), 0) == -1, EOVERFLOW,<br>
+ chkok( altstack(1*MiB, wrap, int2ptr(1000000), NULL) == -1, EOVERFLOW,<br>
getrlimit_|setrlimit_|mmap_|sigaltstack_|sigaction_,<br>
setrlimit_|munmap_|sigaltstack_|sigaction_);<br>
<br>
// be sure segv catch is repeatable (SA_NODEFER)<br>
- chkok( altstack(1*MiB, wrap, int2ptr(1000000), 0) == -1, EOVERFLOW,<br>
+ chkok( altstack(1*MiB, wrap, int2ptr(1000000), NULL) == -1, EOVERFLOW,<br>
getrlimit_|setrlimit_|mmap_|sigaltstack_|sigaction_,<br>
setrlimit_|munmap_|sigaltstack_|sigaction_);<br>
<br>
used = 1;<br>
- chkfail(munmap_, altstack(1*MiB, wrap, int2ptr(1000000), 0) == -1, EOVERFLOW,<br>
+ chkfail(munmap_, altstack(1*MiB, wrap, int2ptr(1000000), NULL) == -1, EOVERFLOW,<br>
getrlimit_|setrlimit_|mmap_|sigaltstack_|sigaction_,<br>
setrlimit_|sigaltstack_|sigaction_);<br>
if (fail = 0, munmap(m_, msz_) == -1)<br>
@@ -150,7 +150,7 @@ int main(void)<br>
ok1(strcmp(buf, estr "\n") == 0);<br>
<br>
used = 1;<br>
- chkok( altstack(8*MiB, wrap, int2ptr(1000000), 0) == -1, EOVERFLOW,<br>
+ chkok( altstack(8*MiB, wrap, int2ptr(1000000), NULL) == -1, EOVERFLOW,<br>
getrlimit_|setrlimit_|mmap_|sigaltstack_|sigaction_,<br>
setrlimit_|munmap_|sigaltstack_|sigaction_);<br>
<br>
@@ -158,7 +158,7 @@ int main(void)<br>
ok1(used >= 8*MiB - pgsz && used <= 8*MiB + pgsz);<br>
<br>
used = 0;<br>
- chkok( altstack(8*MiB, wrap, int2ptr(100000), 0) == 0, 0,<br>
+ chkok( altstack(8*MiB, wrap, int2ptr(100000), NULL) == 0, 0,<br>
getrlimit_|setrlimit_|mmap_|sigaltstack_|sigaction_|munmap_,<br>
setrlimit_|munmap_|sigaltstack_|sigaction_);<br>
<br>
--<br>
2.5.5<br>
<br>
</blockquote></div></div></div>