Christopher Gwilliams
Christopher Gwilliams

Reputation: 1321

C: Passing a variable to a function gives wrong return

I am using some C with an embedded device and currently testing some code to read file details from an SD card. I am using a proprietary API but I will try to remove that wherever possible.

Rather than explaining, I will try to let me code speak for itself:

 char* getImage() {
  int numFiles = //number of files on SD card
  for(int i=0; i<numFiles;i++) {
    \\lists the first file name in root of SD
    char *temp = SD.ls(i, 1, NAMES);
      if(strstr(temp, ".jpg") && !strstr(temp, "_")) {
        return temp;
      }
  }
  return NULL;
}

void loop()
{
  \\list SD contents
  USB.println(SD.ls());
  const char * image = getImage();
  if(image != NULL) {
    USB.println("Found an image!");
    USB.println(image);
    int byte_start = 0;
    USB.print("Image Size: ");
    **USB.println(SD.getFileSize(image));
    USB.println(SD.getFileSize("img.jpg"));**
}

The two lines at the bottom are the troublesome ones. If I pass a literal string then I get the file size perfectly. However, if I pass the string (as represented by the image variable) then I am given a glorious -1. Any ideas why?

For clarity, the print out of image does display the correct file name.

EDIT: I know it is frowned upon in C to return a char and better to modify a variable passed to the function. I have used this approach as well and an example of the code is below, with the same result:

char * image = NULL;
getSDImage(&image, sizeof(image));
void getSDImage(char ** a, int length) {
  int numFiles = SD.numFiles();
  for(int i=0; i<numFiles;i++) {
    char *temp = SD.ls(i, 1, NAMES);
      if(strstr(temp, ".jpg") && !strstr(temp, "_")) {
        *a = (char*)malloc(sizeof(char) * strlen(temp));
        strcpy(*a, temp);
      }
  }
}

EDIT 2: The link to the entry is here: SD.ls and the link for the file size function: SD.getFileSize

From the return, it seems like the issue is with the file size function as the return is -1 (not 0) and because a result is returned when listing the root of the SD.

Thanks!

UPDATE: I have added a check for a null terminated string (it appears that this was an issue) and this has been addressed in the getSDImage function, with the following:

        void getSDImage(char ** a, int length) {
      int numFiles = SD.numFiles();
      for(int i=0; i<numFiles;i++) {
        char *temp = SD.ls(i, 1, NAMES);
          if(strstr(temp, ".jpg") && !strstr(temp, "_")) {
            *a = (char*)malloc(sizeof(char) * strlen(temp));
            strncpy(*a, temp, strlen(temp)-1);
            *a[strlen(*a)-1] = '\0';
          }
      }
}

This seems to work and my results to standard output are fine, the size is now not shown as the error-indicating -1 but rather -16760. I thought I should post the update in case anyone has any ideas but my assumption is that this is something to do with the filename string.

Upvotes: 1

Views: 190

Answers (2)

Bo.
Bo.

Reputation: 2541

There are several things that could be wrong with your code:
1) You might be passing "invisible" characters such as whitespaces. Please make sure that the string you are passing is exactly the same, i.e. print character by character including null termination and see if they are the same.
2) The value that is getting returned by API and latter used by other API may not be as expected. I would advise that (if possible) you look at the API source code. If you can compile the API itself then it should be easy to find the problem (check what API getFileSize() gets from parameters). Based on the API documentation you have sent check the value stored in buffer[DOS_BUFFER_SIZE] after you get -1 from.

EDIT (after looking at the API source code):
On line 00657 (func find_file_in_dir) you have:
if(strcmp(dir_entry->long_name, name) == 0)
it seems as the only reason why you would have different reply when using string literal as opposed to the getting name from your function. So it is very likely that you are not passing the same values (i.e. you are either passing invisible chars or you are missing string termination).
As final note: Check the content of buffer[DOS_BUFFER_SIZE] before each code to SD API.

I hope this helps.

Kind regards,
Bo

Upvotes: 1

unwind
unwind

Reputation: 399703

This:

  if(strstr(temp, ".jpg") && !strstr(temp, "_")) {
    *a = (char*)malloc(sizeof(char) * strlen(temp));
    strcpy(*a, temp);
  }

is broken, it's not allocating room for the terminator and is causing a buffer overflow.

You should use:

*a = malloc(strlen(temp) + 1);

There's no need to cast the return value of malloc() in C, and sizeof (char) is always 1.

Upvotes: 0

Related Questions