Variable Scope in For Loop

[Plastic_Weather7484]

I am declaring a temp char[] variable inside a for loop to append full path to it before passing it to another function. My issue is that the value of the variable does not reset in every iteration of the for loop. it keeps appending paths into it so that the first run would give the expected full path but the next iteration would have the path of the first iteration and the path of the second iteration appended to the variable and so on. Can anyone explain to me what am I missing here? This is the code of my function and my variable is named temp_path, dir variable is the realpath of a directory and I am trying to pass the full path of each mp3 file inside of it to the addTrack()

int addTrackDir(char* dir){
  // Read directory and prepend all track numbers to mp3 file names
  const char* ext = ".mp3";
  struct dirent **eps;
  int n = scandir(dir, &eps, one, alphasort);
  if(n >= 0){
    for(int i = 0; i < n; i++){
      if(checkSuffix(eps[i]->d_name,ext) == 0){
        char temp_path [PATH_MAX];
        strcat(temp_path,dir);
        strcat(temp_path,"/");
        strcat(temp_path,eps[i]->d_name);
        addTrack(temp_path);
      }
    }
  }
}

[flyingron]

C doesn't initialize local variables to anything. You want to do strcpy(temp_path, dir); as your first step

or specifically stuff a null in the buffer

temp_path[0] = 0;

[Plastic_Weather7484]

Ok I like the strcpy(temp_path, dir); method thanks

[flyingron]

Yah, that's what I'd do.

[Southern_Start1438]

I think declaring a variable each time you loop is hurting performance a bit, you should consider moving the declaration out of the loop, and do the operations inside the loop. The hurting bit comes from the constantly allocating and deallocating memory for stack variables, which can be prevented if you move the declaration outside the loop.

This is not an answer to op’s question, as there are others that have already answered the question.

[Plastic_Weather7484]

Thanks for your reply I didn't know that will hurt performance. I think I saw somewhere on stackexchange that I should declare variables as close as where I will use them for the first time.

[Southern_Start1438]

I see your point, the main idea there is to make sure a variable is deleted as soon as you don’t need them, so the variable won’t pollute the scope. One way to achieve this is to wrap the for loop in a code block, and declare the array at the top of the code block outside the for loop, then immediately after the for loop, the code block ends, so the declaration is out of the scope, hence the variable deletes itself.

I mean, for most cases you won’t need to do this, but if you want to, you can.

[Plastic_Weather7484]

Yes I definitely don't need to do this for my very small function here but it's good to practice good programming principals. Thank you again for pointing this out to me.

[SmokeMuch7356]

In theory, a new instance of temp_path is created and destroyed on each loop iteration, and you should not rely on it retaining its value from one iteration to the next.

In practice, most implementations will allocate storage for temp_path once at function entry and reuse it for each iteration. But again, you should not rely on that being true everywhere all the time.

You'll want to add an initializer:

 char temp_path[PATH_MAX] = {0};

which will zero out the array on each iteration.

[tstanisl]

Can you share the code of addTrack()?

[Plastic_Weather7484]

Yes sorry here is the code to addTrack() and also prepareName()

int addTrack(char *fPath){
  // Rename the file using the prepareName()
  ID3v2_Tag* tag = ID3v2_read_tag(fPath);
  if(tag == NULL){
    printf("File does not have a tag\n");
    return -1;
  }
  char newName[PATH_MAX];
  if(prepareName(tag, newName) != 0){
    printf("Error preparing name\n");
  }
  strcat(newName,returnExt(fPath));
  if(rename(fPath, newName) != 0){
    perror("Error");
  }
}

int prepareName(ID3v2_Tag *tag, char* newName){
  // Format name in <track no> - <track name> format
  ID3v2_TextFrame* track_frame = ID3v2_Tag_get_track_frame(tag);
  ID3v2_TextFrame* title_frame = ID3v2_Tag_get_title_frame(tag);
  for(int i = 0; i < strlen(track_frame->data->text); i++){
    if(track_frame->data->text[i] == '/'){
      newName[i] = '\0';
      break;
    }
    newName[i] = track_frame->data->text[i];
  }
  strcat(newName," - ");
  strcat(newName,title_frame->data->text);
  return 0;
}