Structured, No Heap FizzBuzz

Question

Here was my first, simple C fizzbuzz:

#include <stdio.h>
void main() {
  for (int n = 1; n <= 100; ++n) {
    if (n % 3 == 0) printf("%s", "fizz");
    if (n % 5 == 0) printf("%s", "buzz");
    if ((n % 3 != 0) && (n % 5 != 0)) printf("%d", n);
    printf("\n");
  }
}

I was messing around, trying to separate the transformation logic from the looping logic. As a whole, I don't think that my second Buffer-based solution is necessarily "better" as a FizzBuzz solution, but I did like the way that I dealt with a limited-purpose character buffer.

Thoughts on the second solution?

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <assert.h>

#define BUFF_SZ 9

typedef unsigned int u32;
typedef int i32;

/// A string buffer with a notion of its own size.
typedef struct Buffer {
  char *s;
  u32 sz;
  char *ins_ptr;
} Buffer;

/// Returns the content of the buffer.
const char *Buffer_content(Buffer *);

/// Clears contents of the buffer.
void Buffer_clear(Buffer *);

/// Appends a string S to the end of the buffer.
void Buffer_append_str(Buffer *, char *);

/// Appends the string representation of an integer to the end of the buffer.
void Buffer_append_i32(Buffer *, i32);

/// Returns 1 if the buffer has space for a certain number of chars, else 0.
u32 Buffer_has_space(Buffer *, u32);


/* HIGH LEVEL CODE */


/// Writes a number's fizzbuzz transformation into a Buffer.
void write_fizzbuzz_transformation(i32 n, Buffer *buff) {
  Buffer_clear(buff);
  if (n % 3 == 0) Buffer_append_str(buff, "fizz");
  if (n % 5 == 0) Buffer_append_str(buff, "buzz");
  if (*Buffer_content(buff) == '\0') Buffer_append_i32(buff, n);
}

void main() {
  // setup the string buffer (stack allocated)
  char internal_buff[BUFF_SZ];
  Buffer buff;
  buff.s = internal_buff;
  buff.sz = BUFF_SZ;


  for (i32 i = 1; i <= 100; ++i) {
    write_fizzbuzz_transformation(i, &buff);
    printf("%d -> %s\n", i, Buffer_content(&buff));
  }
}


/* DETAILS -- IMPLEMENTATION CODE */


u32 Buffer_has_space(Buffer *buff, u32 requested_sz) {
  u32 available_sz = buff->sz - (buff->ins_ptr - buff->s) - 1;
  return (available_sz >= requested_sz);
}

void Buffer_append_str(Buffer *buff, char* s) {
  u32 needed_sz = strlen(s);
  assert(Buffer_has_space(buff, needed_sz));
  strcpy(buff->ins_ptr, s);
  buff->ins_ptr += needed_sz;
}

void Buffer_append_i32(Buffer *buff, i32 n) {
  i32 original = n;
  u32 needed_sz = 0;
  if (n < 1) ++needed_sz;
  while (n) {
    n /= 10;
    ++needed_sz;
  }
  assert(Buffer_has_space(buff, needed_sz));
  sprintf(buff->ins_ptr, "%d", original);
  buff->ins_ptr += needed_sz;
}

void Buffer_clear(Buffer *buff) {
  buff->ins_ptr = buff->s;
  *(buff->ins_ptr) = '\0';
}

const char *Buffer_content(Buffer *buff) {
  return buff->s;
}

Answer 1

Misleading

Why would You something like this:

typedef unsigned int u32;
typedef int i32;

That's quite misleading. On 64bit machines it's probably not going to be 32 bit wide. Standard uint32_t, int32_t types would be the way to go, if You really need fixed sized integers. Also, typedefing primitive types like that just sucks and kills readability.

Also, You're using unsigned int for storing size. Why not just size_t?

Readability

Typedefing structs as well kills a bit of readability.

What's char *s;? Start? String? Digging through Your code doesn't make it clearer, especially when You have something like this:

void Buffer_clear(Buffer *buff) {
  buff->ins_ptr = buff->s;
  *(buff->ins_ptr) = '\0';
}

What does this mean: if (*Buffer_content(buff) == '\0')? Something like if (Buffer_isEmpty(buff)) would be waaaaay clearer.

Why would You mix casing in function names? void write_fizzbuzz_transformation(), but const char *Buffer_content(), etc.

Also, decide how do You form Your function names. noun_verb, verb_noun, or whatever. Now it's... quite inconsistent. Again, write_fizzbuzz_transformation(), but Buffer_*().

Generic rant

assert() is not intended to be used for production code. It's used only for debugging purposes and only when NDEBUG is defined. Why wouldn't You have proper logger for errors like... not enough space and... wrong input? In fact, You have no control of wrong inputs because none of those are sanitized properly. For example imagine user of Your buffer library does something stupid like Buffer_clear(NULL), Buffer_content(NULL), etc.

In production code You have potential buffer overrun here:

void Buffer_append_str(Buffer *buff, char* s) {
  u32 needed_sz = strlen(s);
  assert(Buffer_has_space(buff, needed_sz));
  strcpy(buff->ins_ptr, s);
  buff->ins_ptr += needed_sz;
}

As I said before, assert() is used only for debugging purposes. You should use strncpy() or check buffer size explicitly. Same applies to void Buffer_append_i32(Buffer *buff, i32 n).

Also, it would be nice if You'd provide proper error codes or 0 on success on Your buffer operations.

Functionality

You could shorten the following and make it safer using snprintf():

u32 needed_sz = 0;
if (n < 1) ++needed_sz;
while (n) {
  n /= 10;
  ++needed_sz;
}
assert(Buffer_has_space(buff, needed_sz));
sprintf(buff->ins_ptr, "%d", original);

You have #define BUFF_SZ 9, but accepting unsigned integers, which are... by Your typedefs --- 32 bit long. So, anything over 99 999 999 goes to abyss...

Warnings

./fizz.c:45:6: warning: return type of ‘main’ is not ‘int’ [-Wmain]
void main() {
     ^