strcalloc_starts() should have tested the return value
of `memcmp` against 0 for equality. Fixed and added a
test case.
As an aside, one might wonder how bugs like that are
creeping into well-tested code imported from other
projects? The answer, specific to stralloc, is that
the original code was very specific to qmail, and used
a number of additional functions specific to qmail.
Rather than import half of qmail, the version imported
into Magicka has been reworked to, instead, use
standard C functions. The process of modifying the
code gave rise to the opportunity for bugs to creep in.
Now that a unit testing framework is in place, we can
test things in isolation more easily and hopefully
catch such things BEFORE they are published to the
master repository.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
Replace unsafe string operations (strcpy, strcat,
sprintf, vsprintf) with safe equivalents:
1. The one use of strcpy into an allocated buffer was
replaced with strdup.
2. The one use of strcat was replaced with a call to
memmove and explicitly setting the NUL terminating
byte.
3. sprintf()/vsprintf() calls were replaced with calls
to snprintf()/vsnprintf(), respectively.
Added a Makefile to build the library as, er, a library
and run the test suite.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
CuTest is a relatively simple unit testing framework for
C code. It is distributed under the zlib license; this
is an import of the pristine sources.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
strcpy()/strcat() are inherently dangerous, even when
used with great care. strlcpy() and strlcat() are
much safer replacements, and are available from OpenBSD
under a very liberal license. Import them and start
using them.
Between pointer vectors, malloz, stralloc and now
strlcpy/strlcat, Magicka has much safer, simpler and
more performant infrastructure for dealing with
strings and dynamic collections of various kinds.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
Lots of code in Magicka is involved in dynamic string manipulation.
`stralloc` isn't a bad library for this sort of thing.
Note that this is complements, but doesn't replace, existing string
utilities.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
Recast more code in terms of the ptr_vector abstraction.
The mail_menu.c code also made a lot of unnecessary copies
of strings. For example, there was this code sequence:
for (i = z; i < lines - 1; i++) {
free(content[i]);
content[i] = strdup(content[i + 1]);
}
free(content[i]);
lines--;
content = (char **)realloc(content, sizeof(char *) * lines);
Here, `content` represents an array of lines of text.
This code is removing an element from somewhere in that
array (possibly in the middle), and then shifting the
remaining elements over one position.
But observe the calls to `free` and `strdup` in the loop
body: the content is already dynamically allocated. We
free whatever was in the selected position, and then make
*another copy* of the data in the next position to put
into the now-available slot in the array: repeat for the
remainder of the array's elements.
Instead, we could change this code to just shift things
down:
free(content[z]);
for (i = z; i < (lines - 1); ++i)
content[i] = content[i + 1];
--lines;
ncontent = realloc(content, sizeof(char *) * lines);
assert(ncontent == NULL);
content = ncontent;
However, the ptr_vector abstraction provides us a function,
`ptr_vector_del` that deletes an element from the array and
returns the pointer, so we can rewrite this as simply:
free(ptr_vector_del(&content, z));
No additional malloc()/free() required, which means less
pressure on the memory allocator and less copying of data.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
More cleaning up construction of arrays of things.
Introduce a utility function called, `split_on_space`
that tokenizes a string on a space character; use
it in most places where `strtok()` had been called.
More use of the ptr_vector type. Introduce a utility
function to get access to the pointers without consuming
the vector; this is used in the files code.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
A repeated pattern in Magicka is to append to dynamically
sized arrays via malloc()/realloc(). Introduce the notion
of a "pointer vector": that is, a growable vector of
pointers, that can be reused to implement that logic more
safely and efficiently (this implementation uses power-of-two
growing).
Many malloc()/realloc() calls were not checked; these
assert() that the return value from realloc() is not NULL.
Add a method to consume the pointer vector: that is, realloc()
it to the current length and return the underlying pointers.
Make the `fmt` argument to dolog() const.
Include <sys/wait.h> in bluewave.c to squash a warning.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
With the normalization of magimail's Makefile,
we can further simplify this logic.
Integrate the WWW logic into GNUmakefile.common.
Remove the custom `Makefile.sunos` files: just
use a conditional in the Makefile.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
With the normalization of magimail's Makefile,
we can further simplify this logic.
Integrate the WWW logic into GNUmakefile.common.
Remove the custom `Makefile.sunos` files: just
use a conditional in the Makefile.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
There were separate `freebsd`, `linux`, `cleanlinux`
and `cleanfreebsd` targets. But these just did the
same thing, so simplify them to just have an `all`
and a `clean` target: this means we have less to plumb
through from the top-level Magicka GNUmakefile.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
Delegate most of the logic to a "common" GNUmakefile,
with each system-specific GNUmakefile only setting a
handful of necessary variables.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
Simplify the logic around making connections in the
chat system by delegating to utility functions that
return early on failure.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
