The Boston Diaries

The ongoing saga of a programmer who doesn't live in Boston, nor does he even like Boston, but yet named his weblog/journal “The Boston Diaries.”

Go figure.

Tuesday, January 04, 2022

Fixing bugs by cleaning code

I figured out an approach to “Project: Bradenburg.” I'm dumping the C++ code as it was never used in production and as someone who is more confortable with C than C++, I think that's the best choice right now. To that end, I've made a checklist of the items that need addressing right now—and they're all related to cleaning up the code.

I've normalized the whitespace in all the source files. Tab characters were introduced by the previous developer, who used a different tab stop setting than I'm used to, so the code formatting is just way off. I removed the #pragma declarations and fixed all the resulting warnings. And for the warnings I don't necessarily agree with, like -Wformat-truncation, I can supress in the makefile (well, GNUmakefile if you want to be pedantic):

src/app/stormgr.o : override CFLAGS += -Wno-format-truncation

This disables the warning

src/app/stormgr.c:194:3: note: ‘snprintf’ output between 2 and 4097 bytes into a destination of size 4096

from code like this:

snprintf(filename, sizeof(filename),"%s/", GetGlobalConfig()-­>spool_dir);

It's nice I get the warning, but in this case, if we get a filename of 4097 characters long we have other issues to worry about.

And that's pretty much the only warning I have to supress. The other warnings were all issues that needed to be addressed, and in one case, an actual bug was fixed.

Right now I'm cleaning up what is, to me, a pointless abstraction dealing with syslog(). The previous developer wrote a wrapper around syslog() called SysLog(). It would be one thing if there was some reason to wrap syslog(), like we would be running on Windows (we won't) or needed to redirect output to a file (we don't). But no, all the wrapper does is:

void SysLog(int priority, const char *fmt, ...)
{
  va_list marker;
    
  if (fSyslogOpened)
  {
    va_start(marker, fmt);
    vsyslog(priority, fmt, marker);
    va_end(marker);
  }
}

And the funny thing—syslog() already handles the case when openlog() hasn't been called. So there's no reason for this wrapper whatsoever in this code base. What makes this even more special is how the developer called SysLog():

SysLog(LOG_NOTICE, BRADENBURG5024,  dbuf);

And elsewere in the code in some header file:

#define BRADENBURG5024    "BRADENBURG5024: OUTBIND connection accepted from %s"

I … um … I … erm … yeah. I can make sense of this, in the “we might want to translate the log messages to another language” argument. But we don't sell this product—it's for internal use. And there are other ways to go about doing this rather than separate the format string from its use. Nothing like finding nine instances of this:

SysLog(LOG_EMERG, BRADENBURG0001);

where BRADENBURG0001 is defined as:

#define BRADENBURG0001    "BRADENBURG0001: %s"

For those unwise in the ways of C programming, this is calling a function with effectively a missing parameter and the compiler can't warn about it because the format string (which informs the called function about what parameters to expect) exists in a different file.

So just by cleaning up the code and removing pointless abstractions I'm fixing bugs.

Obligatory Picture

[It's the most wonderful time of the year!]

Obligatory Contact Info

Obligatory Feeds

Obligatory Links

Obligatory Miscellaneous

You have my permission to link freely to any entry here. Go ahead, I won't bite. I promise.

The dates are the permanent links to that day's entries (or entry, if there is only one entry). The titles are the permanent links to that entry only. The format for the links are simple: Start with the base link for this site: http://boston.conman.org/, then add the date you are interested in, say 2000/08/01, so that would make the final URL:

http://boston.conman.org/2000/08/01

You can also specify the entire month by leaving off the day portion. You can even select an arbitrary portion of time.

You may also note subtle shading of the links and that's intentional: the “closer” the link is (relative to the page) the “brighter” it appears. It's an experiment in using color shading to denote the distance a link is from here. If you don't notice it, don't worry; it's not all that important.

It is assumed that every brand name, slogan, corporate name, symbol, design element, et cetera mentioned in these pages is a protected and/or trademarked entity, the sole property of its owner(s), and acknowledgement of this status is implied.

Copyright © 1999-2022 by Sean Conner. All Rights Reserved.