Problem with inserting a character into a string array. STUMPED!

[wwatson]

This one has me stumped.

Here is the code:

#include "Arduino.h"

#define CNT_PARITIONS 24

// ----------------------------------------------------------------------------
// Find a device and return the logical drive index number for that device and
// the path spec with volume label stripped off of the path spec.
// param in:  full path.
// param out: device index number. Path name stripped of volume label.
// ----------------------------------------------------------------------------
int getLogicalDeviceNumber(const char **path) {
  const char *tempPath;
  char tempChar, ldNumber[256];
  const char *strPtr;
  char pathChar;
  int i = 0, volume = -1;
  uint8_t cntDigits = 0;
	
  tempPath = *path;
  if (!tempPath) return volume;	// Invalid path name?
	
  // Check if using logical device number ("0:" etc...).
  sprintf(ldNumber,"%s",*path); 
  // Look for a colon in the path spec. Terminate on end of string
  // or if colon found.
  do {
    tempChar = ldNumber[cntDigits];
    cntDigits++; // Inc digit count for atoi().
  } while ((uint8_t)tempChar != '\0' && tempChar != ':');
  if (tempChar == ':') {	// Is this a volume number?
    cntDigits--;		// Backup to ':' position.
[COLOR="#00FF00"]  ldNumber[cntDigits] = '/';  // Change ':' to '/'.[/COLOR]
    i = atoi(ldNumber); // numeric value of number string.
    if (i < CNT_PARITIONS) {	// Don't overrun array.
      volume = i;		// Drive number
[COLOR="#FF0000"]//  ldNumber[cntDigits] = '/';  // Change ':' to '/'.[/COLOR]
[COLOR="#FF0000"]//  delay(1);[/COLOR]
    *path = ldNumber+cntDigits;	// Snip off the drive number.
    }
    return volume;
  }
  // Look for logical device name (/volume name/).
/*
	if (*tempPath == '/') { // Look for first '/'
		do {
			strPtr = drvIdx[i].name;	// Volume label.
			tempPath = *path;			// Path to search.
			// Compare the volume label with path name.
			do {
				// Get a character to compare (with inc).
				pathChar = *strPtr++; tempChar = *(++tempPath);
				if (ifLower(pathChar)) pathChar -= 0x20; // Make upper case.
				if (ifLower(tempChar)) tempChar -= 0x20; // Ditto.
			} while (pathChar && (char)pathChar == tempChar);
		  // Repeat for each label until there is a pattern match.
		} while ((pathChar || (tempChar != '/')) && ++i <= CNT_PARITIONS);
		// If a volume label is found, get the drive number and strip label from path.
		if (i <= CNT_PARITIONS) {
			volume = i;		// Volume number.
			*path = tempPath; // Strip off the logical drive name (leave last '/').
			return volume;
		}
	}
	if(*tempPath == '/')
		return volume;	// Return error (-1). 
	return currDrv;
*/
  return volume;
}

// Change '32GSDFAT32' to a volume name of your drives.
//const char *device = "/TEENSY8GSD/test1.txt";
const char *device = "16:test1.txt";

void setup()
{
  // Open serial communications and wait for port to open:
  Serial.begin(9600);
   while (!Serial);
  int ldn = getLogicalDeviceNumber(&device);
  Serial.printf("ldn = %d, device = %s\n",ldn, device);
}

void loop() {
}

The ouput from the above code is:

ldn = 16, device = [COLOR="#00FF00"]/[/COLOR]test1.txt

This is good.

Now if I comment out the green line and un-comment the the first red line I get This:

ldn = 16, device = [COLOR="#FF0000"]:[/COLOR]test1.txt

This is wrong.

If I un-comment the second red line that is the delay I get this:

ldn = 16, device = [COLOR="#00FF00"]/[/COLOR]test1.txt

This is also correct but why would I need the delay

I have been hammering on this for almost a whole day. Any ideas what I am missing?

 

[MarkT]

You are returning a pointer to a local array from inside the function via the path pointer. This is undefined, the storage for
ldNumber is reclaimed when getLogicalDeviceNumber returns.

I think the strange change in behaviour you are seeing is because the compiler is optimizing your code under
assumptions that are not true - you broke the rules, the compiler is allowed to do anything.
The delay() call is splitting the basic-block structure in the code that compilers typically use in their analysis,
which is why the behaviour can be suddenly different. https://en.wikipedia.org/wiki/Basic_block

You'll likely get different behaviour if you change compiler optimization levels, but your code is
broken till you fix the dangling pointer issue.

 

[wwatson]

Thanks for the response. I knew it was my lack of programming know how. And thanks for the link

UPDATE:
@MarkT - Here are my changes to the sketch that works:

#include "Arduino.h"

#define CNT_PARITIONS 24
#define ifLower(c)		((c) >= 'a' && (c) <= 'z')

// ----------------------------------------------------------------------------
// Find a device and return the logical drive index number for that device and
// the path spec with volume label stripped off of the path spec.
// param in:  full path.
// param out: device index number. Path name stripped of volume label.
// ----------------------------------------------------------------------------
int getLogicalDeviceNumber(char *path) {
	char *tempPath;
	char tempChar, ldNumber[256];
	const char *strPtr;
	char pathChar;
	int i = 0, volume = -1;
	uint8_t cntDigits = 0;
	
	tempPath = path;
	if (!tempPath) return volume;	// Invalid path name?
	
	// Check if using logical device number ("0:" etc...).
	sprintf(ldNumber,"%s",path); 
	// Look for a colon in the path spec. Terminate on end of string
	// or if colon found.
	do {
		tempChar = ldNumber[cntDigits];
		cntDigits++; // Inc digit count for atoi().
	} while ((uint8_t)tempChar != '\0' && tempChar != ':');
	if (tempChar == ':') {	// Is this a volume number?
		cntDigits--;		// Backup to ':' position.
		i = atoi(ldNumber); // numeric value of number string.
		if (i < CNT_PARITIONS) {	// Don't overrun array.
			volume = i;		// Drive number
			ldNumber[cntDigits] = '/';  // Change ':' to '/'.
			sprintf(path, "%s", ldNumber+cntDigits);	// Snip off the drive number.
		}
		return volume;
	}
    // Look for logical device name (/volume name/).
/*
	if (*tempPath == '/') { // Look for first '/'
		do {
			strPtr = drvIdx[i].name;	// Volume label.
			tempPath = path;			// Path to search.
			// Compare the volume label with path name.
			do {
				// Get a character to compare (with inc).
				pathChar = *strPtr++; tempChar = *(++tempPath);
				if (ifLower(pathChar)) pathChar -= 0x20; // Make upper case.
				if (ifLower(tempChar)) tempChar -= 0x20; // Ditto.
			} while (pathChar && (char)pathChar == tempChar);
		  // Repeat for each label until there is a pattern match.
		} while ((pathChar || (tempChar != '/')) && ++i <= CNT_PARITIONS);
		// If a volume label is found, get the drive number and strip label from path.
		if (i <= CNT_PARITIONS) {
			volume = i;		// Volume number.
			path = tempPath; // Strip off the logical drive name (leave last '/').
			return volume;
		}
	}
	if(*tempPath == '/')
		return volume;	// Return error (-1). 
	return currDrv;
*/
return volume;
}

// Change '32GSDFAT32' to a volume name of your drives.
//char device[256] = {"/TEENSY8GSD/test1.txt"};
char device[256] = {"16:test1.txt"};

void setup()
{
  // Open serial communications and wait for port to open:
  Serial.begin(9600);
   while (!Serial);
  int ldn = getLogicalDeviceNumber(device);
  Serial.printf("ldn = %d, device = %s\n",ldn, device);
}

void loop() {
}

Output:

ldn = 16, device = /test1.txt

Thanks