Small typo in 'afterburner.c'

[hubertushirsch]

In line 188 in the printHelp() function the serial baud rate used is incorrectly specified. The help text says 38400 but the code uses 57600.

In addition in printHelp(), the default device (if no -d 'device' option is specified) is not always COM1 (on Windows), but the highest COM number found (since the extension in serial_port.h).

Hubert

[ole00]

Thanks for the report. It should be now fixed. See commit 0761907

[hubertushirsch]

Hi, thanks for the fix.

I have now tried the following change. It works on Windows. I don't have a Linux system to test it with.

static void printHelp() {
    if (!deviceName) {
        serialDeviceGuessName(&deviceName);
    }
    printf("Afterburner " VERSION_EXTENDED "  a GAL programming tool for Arduino based programmer\n");
    .....
    printf("  -d <serial_device> : name of the serial device. Without this option the device %s is tried.\n", deviceName ? deviceName : DEFAULT_SERIAL_DEVICE_NAME);
    printf("                       serial params are: 57600, 8N1\n");
    .....

In your code I keep noticing the inconsistent use of the '0' value. This contradicts the rules of C/C++ syntax and the program is also more difficult to understand for co-programmers. Even if it technically works.

0 is the value zero for a numeric variable. If you don't want a pointer to point to a valid memory location (variable), you assign it the value NULL or test for NULL if you want to know whether the pointer points to a valid memory location. For char variables or char arrays, '\0' should be used for the null character.

e.g. in afterburner.c

char verbose = 0;
char* filename = 0;
char* deviceName = 0;
char* pesString = NULL;

should be

char verbose = 0;
char* filename = NULL;
char* deviceName = NULL;
char* pesString = NULL;

or in serial_port.h

  //strip the new-line trailing markers
  if (guessedSerialDevice[textLen - 1] == '\n' || guessedSerialDevice[textLen - 1] == '\r') {
      guessedSerialDevice[textLen - 1] = 0;
  }

should be

 //strip the new-line trailing markers
  if (guessedSerialDevice[textLen - 1] == '\n' || guessedSerialDevice[textLen - 1] == '\r') {
      guessedSerialDevice[textLen - 1] = '\0';
  }

because guessedSerialDevice is a char array that should then be initialized like this:

char guessedSerialDevice[512] = {'\0'};

Greetings

[hubertushirsch]

I forgot to comment the line

char verbose = 0;

verbose is used in the program as a pure YES/NO flag. So verbose doesn't actually contain a character as a value. For better understanding, it is a Boolean variable that only has the two values true and false. You could also write it like this for better readability:

#include <stdbool.h>
bool verbose = false;
.....
  } else if (strcmp("-v", param) == 0) {
    verbose = true;
.....

This also applies to functions that you declared as char, which only return 0 or 1 as a result for successful or failed.

bool function(args) {       // instead of char function(args)
   ....
   return true; // or false // instead of return 1; // or 0
}

Greetings Hubert

[Emile666]

Very good comments! While adding them to my local files, I found the following bug: L.638: "if (verbose & bigRam) {", this should be: "if (verbose && bigRam) {". A single & is a bitwise operator.