HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

C Development Utilities Library

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
librarydevelopmentutilities

Problem

I have been working on a small library of development utilities for some time now, incrementally improving and expanding it as I make use of it in other projects. I feel that it is fairly mature now; I've worked out enough bugs that I can say it is decently stable and I feel that it is time for a code review on what I've created so far to make sure everything is kosher.

It has an implementation of better strings and string buffers, a unit testing framework, atomic code profiling framework, cleaner assert-or-fail style checks with a customizable error logging system, and safer memory management utilities.

In any case, here's the repository: https://github.com/daltonwoodard/cdevkit

I'll also post the API's for several of the sections. There are demos of the unit testing framework and code profiling framework in the cdevkit/demos/ directory.

API:

safemem.h

```
#ifndef __safemem_h_
#define __safemem_h_

#include
#include

#include "check.h"
#include "cdk_stack.h"

//
// Memory allocation stats
//
#ifdef TRACK_MEM
static unsigned long int __sm_bytes_allocated_c = 0; // current
static unsigned long int __sm_bytes_allocated_t = 0; // total
static unsigned long int __sm_bytes_allocated_p = 0; // peak

#define get_bytes_allocated_current() __sm_bytes_allocated_c
#define get_bytes_allocated_total() __sm_bytes_allocated_t
#define get_bytes_allocated_peak() __sm_bytes_allocated_p

#define mem_stats_add( bytes ) __sm_bytes_allocated_c += (bytes); \
__sm_bytes_allocated_t += (bytes); \
__sm_bytes_allocated_c > __sm_bytes_allocated_p ? __sm_bytes_allocated_p = __sm_bytes_allocated_c \
: __sm_bytes_allocated_p += 0;

#define mem_stats_sub( bytes ) __sm_bytes_allocated_c -= (bytes);
#endif

//
// Pushes a new pointer to the register stack and returns the pointer.
// Not

Solution

Standards


Note also that this project is developed to be compliant with the C11 standard.

Cool, let's find non-standard behavior.

The first thing that jumps out to me is your usage of double underscores.

Both C99 and C11 say (in section 7.1.3):


All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use.

You include ` everywhere. The only operating system where I could find that header file is MacOS. This is definitely not standard.

You seem to be very dependent on the function
malloc_size in many places. As far as I know, only MacOS implements it.

You use the function
valloc. This is a non-standard function that has been marked as obsolete in 4.3 BSD in 1986. You can get this functionality from a standard (not C, but at least POSIX) by using the posix_memalign function.

Problematic behavior

In
cstring_cmp and cstringbuffer_cmp you compare with the shortest string with strncmp which means that you're only comparing the prefixes of the strings (I think, I haven't dug into the exact details of the library) and why not just use memcmp since you're not comparing C strings anyway.

It's possible to write
\0 into a cstringbuffer with cstringbuffer_write_char (and other functions) that will update the length to include that and later append further characters at the end of the buffers, but you're then using normal strcmp, strncpy, etc. to compare and copy the strings which will not do what you want. Either disallow \0 explicitly or use functions that don't get confused by it.

allocate_mem has a good prototype with count and size. This is just like calloc and it's what a modern memory allocator should do. But, you miss the point of why calloc does it. The main reason is that you can detect overflows when multiplying count * size. You don't check for overflow in that multiplication which can lead to security issues.

Matters of taste

You're always allocating a struct on the stack, then malloc the same struct, then return a copy. Why? It seems like extra work.

Macros with magic
goto error; with the error: label being outside of the macro are horrifying.

You do
do { ... } while (0) in macros, this is good style. But you then defeat half the purpose of building macros this way by adding the ; inside the macro. for example if (foo) log_error("foo"); else do_something();` will not compile.

Context

StackExchange Code Review Q#55275, answer score: 4

Revisions (0)

No revisions yet.