In bluewave.c mostly. There are a few places left where sprintf()
is called directly; these should be recast in terms of a stralloc
or possibly strlcat.
One small whitespace change in www_files.c.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
Replace most remaining uses of sprintf() into a `buffer`
variable followed by realloc() and strcat() with direct
use of stralloc.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
Specifically, change the www_last10 HTML rendering logic
to use stralloc and strftime(). This eliminates a lot of
duplication.
It would be easier to test this with a unit test if the
logic of reading the last10 entries from a file were
separated from the HTML rendering logic. An area for
future enhancement.
Also start in on www_email.c, which is the last bastion
of significant realloc() use for page generation. An
explicit goal is to get rid of unsafe string handling
functions such as strcpy, strcat, sprintf, etc.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
strcat()'ing a string onto the result of file2str()
will result in a buffer overflow, since file2str()
only allocates enough memory to hold the contents of
the file (plus a NUL terminator). This happend in
`bluewave.c`.
Instead, use `file2stralloc` to read the contents of
that file into a stralloc, which we can stralloc_cats
onto without fear of overflow.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
Make `blog_load` return a ptr_vector which is
consumed by the code that uses blog entries.
Greatly clean up WWW page generation by using
stralloc and strftime and the ptr_vector
infrastructure.
Needs to be tested. :-)
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
The pointer vector maintaining `content` in `editor` already
keeps track of the number of lines and makes it available via
a call to `ptr_vector_len` (or one could look at th `len`
member of the ptr_vector struct...this is C, not some fancy
object oriented language with data hiding). Delete the `lines`
local variable and just use ptr_vector_len where necessary.
Sorry; I should have done that in the first sweep through that
code. My bad!
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
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>
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>
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>
