Starting with step 36, an "append buffer" implementation is presented, starting with these declarations:
struct abuf { char *b; int len; }; #define ABUF_INIT {NULL, 0}
First of all, abuf is a exceedingly short and non-specific name to be putting in the global namespace. I know this is meant to be a relatively short program, but it's an old proverb in software development that big programs start their lives as small programs. I'm also not a huge fan of using the preprocessor to abstract away the initialization of a struct abuf value (I really don't like using the preprocessor at all when it can be avoided.) A compiler with a decent optimizer should generate comparable code for a call to something like this:
static inline void abInit(struct abuf* ab) { ab->b = NULL; ab->len = 0; }
Another question I have about this implementation is, why use a dynamic buffer at all? As is shown in later steps, this is used to buffer output to the screen, instead of writing lots of short strings one at a time. It remains to be seen (by me, anyway) if this is its only use this data structure has, but it seems to me a relatively small fixed-size buffer would serve as well, without putting load on the dynamic allocation system:
struct abuf { char b[1024]; int len; };
I'd also point out that I'd really like to see clearing of the struct abuf fields in abFree:
void abFree(struct abuf& ab) { free(ab->b); ab->b = NULL; ab->len = 0; }
Leaving these with their old values is another bug waiting to happen. Finally, I'd add these utility abstraction functions to clarify the calling code:
static inline void abAppendStr(struct abuf* ab, const char* string) { abAppend(ab, string, strlen(string)); } static inline void abWrite(struct sbuf* ab) { write(STDOUT_FILENO, ab->b, ab->len); }
Note the use of static inline, so this adds a very helpful abstraction layer with zero cost at runtime.
No comments:
Post a Comment