Initial commitHEADmaster

Signed-off-by: Andrew Clayton <ac@sigsegv.uk>

7 files changed, 292 insertions, 0 deletions

diff --git a/.gitignore b/.gitignore
new file mode 100644
index 0000000..a270ec1
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1,4 @@
+/examples/*
+!/examples/*.c
+
+some_common_c_mistakes.html
diff --git a/Makefile b/Makefile
new file mode 100644
index 0000000..f91cbcb
--- /dev/null
+++ b/Makefile
@@ -0,0 +1,5 @@
+html: some_common_c_mistakes.md
+	pandoc $< --metadata pagetitle=some-common-c-mistakes -s --highlight-style tango -o some_common_c_mistakes.html
+
+clean:
+	rm -f some_common_c_mistakes.html
diff --git a/examples/empty-func-args.c b/examples/empty-func-args.c
new file mode 100644
index 0000000..8093c22
--- /dev/null
+++ b/examples/empty-func-args.c
@@ -0,0 +1,4 @@
+void f()
+{
+	/* Do something */
+}
diff --git a/examples/malloc-cast.c b/examples/malloc-cast.c
new file mode 100644
index 0000000..5015147
--- /dev/null
+++ b/examples/malloc-cast.c
@@ -0,0 +1,4 @@
+void f(void)
+{
+	int *vals = malloc(sizeof(int) * 10);
+}
diff --git a/examples/pass-by-value.c b/examples/pass-by-value.c
new file mode 100644
index 0000000..c452591
--- /dev/null
+++ b/examples/pass-by-value.c
@@ -0,0 +1,41 @@
+#include <stdio.h>
+#include <stdlib.h>
+
+struct s {
+	int i;
+};
+
+static void fv(struct s *s)
+{
+	/* Do something with s */
+
+	free(s);
+	s = NULL;
+}
+
+static void fr(struct s **s)
+{
+	/* Do something with s */
+
+	free(*s);
+	*s = NULL;
+}
+
+int main(void)
+{
+	struct s *s;
+
+	s = malloc(sizeof(*s));
+	printf("s: %p\n", s);
+	fv(s);
+	printf("s: %p\n", s);
+
+	printf("\n");
+
+	s = malloc(sizeof(*s));
+	printf("s: %p\n", s);
+	fr(&s);
+	printf("s: %p\n", s);
+
+	return 0;
+}
diff --git a/examples/strsep-invalid-free.c b/examples/strsep-invalid-free.c
new file mode 100644
index 0000000..e01685d
--- /dev/null
+++ b/examples/strsep-invalid-free.c
@@ -0,0 +1,16 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+int main(void)
+{
+	char *str = strdup("Hello World"), *tkn;
+
+	printf("str [%s@%p]\n", str, str);
+	tkn = strsep(&str, " ");
+	printf("tkn [%s] str [%s@%p]\n", tkn, str, str);
+
+	free(str);
+
+	return 0;
+}
diff --git a/some_common_c_mistakes.md b/some_common_c_mistakes.md
new file mode 100644
index 0000000..512b045
--- /dev/null
+++ b/some_common_c_mistakes.md
@@ -0,0 +1,218 @@
+``` {=html}
+<style>
+body { max-width: 48em !important; }
+</style>
+```
+
+##### [digital-domain.net](https://digital-domain.net/)
+
+# Some common C mistakes
+
+First of all I'm talking specifically about C, _not_ C++. However some of this
+_may_ also apply to C++.
+
+## C only supports pass-by-value
+
+C really only supports pass by value. Pass by reference can be accomplished
+by using pointers.
+
+A fairly common thing I see in codebases is something like the following
+
+```C
+#include <stdio.h>
+#include <stdlib.h>
+
+struct s {
+        int i;
+};
+
+static void f(struct s *s)
+{
+        /* Do something with s */
+
+        free(s);
+        s = NULL;
+}
+
+int main(void)
+{
+        struct s *s = malloc(sizeof(*s));
+
+        printf("s: %p\n", s);
+        f(s);
+        printf("s: %p\n", s);
+
+        return 0;
+}
+```
+
+Which produces the following output
+
+```
+$ ./pass-by-value
+s: 0x8092a0
+s: 0x8092a0
+```
+
+In the function _f()_ we pass a pointer to a struct. Something is done with
+_s_, then it's free(3)'d.
+
+So far, so good, but then a common thing to do is to _NULL_ the free'd
+pointer.
+
+However that will have no effect outside _f()_. The pointer address was passed
+to _f()_ on the stack and now has no direct connection to _s_ in _main()_.
+
+If you want to really _NULL_ out the pointer, then you need to pass it in by
+_reference_ by passing in a pointer to it. E.g
+
+```C
+static void f(struct s **s)
+{
+        /* Do something with *s */
+
+        free(*s);
+        *s = NULL;
+}
+```
+
+then in _main()_ we pass in _s_ like
+
+```C
+        f(&s);
+```
+
+which now produces
+
+```console
+$ ./pass-by-value
+s: 0x8092a0
+s: (nil)
+```
+
+## Function prototypes with no arguments
+
+It is quite common to see things like
+
+```C
+void f()
+{
+        /* Do something */
+}
+```
+
+where _f()_ takes no arguments. Writing it like the above has different
+meanings in C and C++.
+
+In C++ it means a function that takes no arguments. In C it means the number
+(and type) of arguments hasn't been specified (note, this is different to
+using '...' to specify a variable number of arguments).
+
+The _correct_ way to do this in C is
+
+```C
+void f(void)
+{
+        /* Do something */
+}
+```
+
+You can enable -Wstrict-prototypes in both _gcc_ and _clang_ to catch these.
+E.g
+
+```console
+$ gcc -Wstrict-prototypes -c empty-func-args.c
+empty-func-args.c:1:6: warning: function declaration isn’t a prototype [-Wstrict-prototypes]
+    1 | void f()
+      |      ^
+```
+
+```console
+$ clang -Wstrict-prototypes -c empty-func-args.c
+empty-func-args.c:1:7: warning: this old-style function definition is not preceded by a prototype [-Wstrict-prototypes]
+void f()
+      ^
+1 warning generated.
+```
+
+## Casting the return type of malloc et al
+
+OK, so this one might be slightly controversial...
+
+Another common thing I see is code like
+
+```C
+        int *vals = (int *)malloc(sizeof(int) * nr_vals);
+```
+
+Note the casting of the return from malloc(3). While this may be required in
+C++, in C
+
+ - It's superfluous. Why clutter the code with unnecessary casts?
+
+ - It can actually hide bugs. OK, how you might ask...
+
+Well lets say you forgot to
+
+```C
+#include <stdlib.h>
+```
+
+so you don't have the declaration of malloc(3), now the compiler may assume
+that the return type of malloc(3) is an int. Now you may be getting a
+truncated memory address returned.
+
+NOTE: With current compilers this is not such a problem anymore and you will
+get a clear warning about the missing declaration. But the first point still
+stands.
+
+## strsep(3) and invalid free(3)
+
+Lets take the following example
+
+```C
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+int main(void)
+{
+        char *str = strdup("Hello World"), *tkn;
+
+        printf("str [%s@%p]\n", str, str);
+        tkn = strsep(&str, " ");
+        printf("tkn [%s] str [%s@%p]\n", tkn, str, str);
+
+        free(str);
+
+        return 0;
+}
+```
+
+When run we get the following
+
+```console
+$ ./strsep-invalid-free
+str [Hello World@0xf77e2a0]
+tkn [Hello] str [World@0xf77e2a6]
+free(): invalid pointer
+Aborted (core dumped)
+```
+
+This is due to *str* being modified, as you can see its address changes. Thus
+when it’s passed to free(3) *str* is no longer pointing to the address that
+was originally allocated.
+
+Rather you need to take a *copy* of the *str* pointer and pass that to
+free(3), e.g.
+
+```C
+        char *p;
+        ...
+        p = str;
+        /* Do strsep(3)'s */
+        free(p);
+```
+
+Similarily if you set *str* to *NULL* in strsep(3) or strtok(3) watch you
+don't end up just doing *free(NULL)*.