Error while trying to list folders

Hi, i am currently making a project where a part of the project is supposed to send a list of files and folders in the "current directory" (first argument, declared earlier in the project) to the socket "ControlServer"; however im currently only recieving a partially successfull result. It sends "C:\" correctly but if i change directory to for example "C:\Temp\" it will either send nothing or 'something' that is probably binary like "".

Here is the part of the project code that is relevant:

Code:

---

void getfolds(char *argz)
{

//prepare variables
    char argv[sizeof(argz)+4];
    memcpy(argv, argz,sizeof(argz));
int counterf=0;
int counterd=0;
string output;
string files;
string folders;
HANDLE searchab;
char last[1024];
char current[1024];
strcat(argv,"\\*");
WIN32_FIND_DATAA file;

//do the search
searchab = FindFirstFile(argv, &file);
do{
FindNextFile(searchab, &file);
strcpy(last,current);
strcpy(current,file.cFileName);
if(strcmp(current, last) != 0 && strcmp(current, "..") != 0){
if(file.dwFileAttributes  == 0x10){
if(counterd > 0)folders += "\r\n";
folders += file.cFileName;
counterd++;
}else{
if(counterf > 0)files += "\r\n";
files += file.cFileName;
counterf++;
}
}
} while (strcmp(current, last) != 0);

//paste together and send to teh interwebs
output += "cd ";
output += folders;
output += "\r\n";
output += files;
send(ControlServer,output.c_str(),strlen(output.c_str()),0);
}

---

I am sure the socket works since a whole lot of other functions that use it work perfectly (including openening a file and sending the content over the socket); but i dont know why it refuses to send the folders correctly (might it be that i dont have the privilegies to see the folders?) It has been tested on Windows 7-64 bit and XP-32 bit. I am currently using dev-cpp with MinGW.

Thanks in advance =)

edit: I think it must be something with the lines

Code:

---

char argv[sizeof(argz)+4];
    memcpy(argv, argz,sizeof(argz));

---

(when current directory is "C:\temp" it acctually searches "C:\tå@\*" (4 characters before random stuff)

edit2: think i fixed it by editing the first couple of lines into

Code:

---

char argv[strlen(argz)+4];
    strcpy(argv, argz);

---

There's still several things wrong.

1. Indent. It's only 30 lines, and it's already nearly impossible to follow.

2. You ignore the result of the findfirst call, presumably you're assuming that the first call returns ".".

3. You ignore the status result of findfist/findnext. This would be more robust than relying on the strcmp to detect whether the failing call modifies the result (or not).

4. char argv[strlen(argz)+4];
This isn't valid C or C++ code. You're relying on a compiler extension.

5. if(file.dwFileAttributes == 0x10)
Directories can have other attributes as well, say hidden.

Quote:

---

Originally Posted by Salem

There's still several things wrong.

1. Indent. It's only 30 lines, and it's already nearly impossible to follow.

---

Was just supposed to be something i hacked in to get it working and i know this does NOT have a beautiful syntax; that is something i still need to learn. However did add comments to help follow code.

Quote:

---

Originally Posted by Salem

2. You ignore the result of the findfirst call, presumably you're assuming that the first call returns ".".

---

I have always recieved the correct result this way; however i realise (afer reading on msdn) that this was just me being lucky with what the system sent me; will be fixed after this post is written.

Quote:

---

Originally Posted by Salem

3. You ignore the status result of findfist/findnext. This would be more robust than relying on the strcmp to detect whether the failing call modifies the result (or not).

---

Added a if statement to check if it was sucessfull

Quote:

---

Originally Posted by Salem

4. char argv[strlen(argz)+4];
This isn't valid C or C++ code. You're relying on a compiler extension.

---

why isn't this valid c++ code? I thought strlen was a standard function and i want to reserve the return value of strlen() +4 chars so should work fine or..?

Quote:

---

Originally Posted by Salem

5. if(file.dwFileAttributes == 0x10)
Directories can have other attributes as well, say hidden.

---

i know and did know i failed here but was just going to see if it worked to sort out directories and files; will add the other attributes later

New Code

Code:

---

void getfolds(char *argz)
{
char argv[(strlen(argz)+4)];
strcpy(argv, argz);
int counterf=0;
int counterd=0;
string output;
string files;
string folders;
HANDLE searchab;
char last[1024];
char current[1024];
strcat(argv,"\\*");
WIN32_FIND_DATAA file;
searchab = FindFirstFile(argv, &file);


do{ /*Start to do through the directories*/
if(FindNextFile(searchab, &file)!=0){
strcpy(last,current);
strcpy(current,file.cFileName);
if(strcmp(current, last) != 0 && strcmp(current, "..") != 0){ //Check if new result is the same as last one AND check if result is ".."
if(file.dwFileAttributes  == 0x10){ //If it is a folder (dosen't include other attributes yet like hidden, to be improved)
if(counterd > 0)folders += "\r\n"; //if more then 1 directory is found, add \r\n before it (new line)
folders += file.cFileName;
counterd++;
}else{ //else if not a folder (assuming file, but can be hidden folders or 'other' things; to be improved)
if(counterf > 0)files += "\r\n";  //if more then 1 file, add \r\n
files += file.cFileName;
counterf++;
} //end of "if file or folder"
} //end of check if result is same or ".."
} else {//end of "if FindNextFile was successfull" and what to do if it wasn't
      if(GetLastError() != ERROR_NO_MORE_FILES){ //check if error == no more files
      send(ControlServer,"Error while listing files\r\n",28,0);}
      break;
      }
} while (strcmp(current, last) != 0); //check if last and current is same and go out of loop if it is
/*This part should add both folders (first) and files (last) to output. Should also add "cd " at the start now (debugging, may be removed later)*/
output += "cd ";
output += folders;
output += "\r\n";
output += files;
send(ControlServer,output.c_str(),strlen(output.c_str()),0);
}

---

Other than that, thanks a lot for your reply; I have been programming for myself a while now and need some feedback on my code. Again thanks
~Hellbinder

> Was just supposed to be something i hacked in to get it working and i know this does NOT have a beautiful syntax;
Except most people go and do something more enjoyable, like visit the dentist, than wade through a page of unformatted code.
You're competing with others for a finite amount of attention time, and unindented code counts against you.

Besides, if you format to begin with, you'll have a much easier time yourself as well.

> why isn't this valid c++ code?
Only C99 supports variable length arrays.
MinGW (actually GCC underneath) supports variable length arrays in C++ as an extension.

Quote:

---

Originally Posted by Salem

>
Except most people go and do something more enjoyable, like visit the dentist, than wade through a page of unformatted code.
You're competing with others for a finite amount of attention time, and unindented code counts against you.

Besides, if you format to begin with, you'll have a much easier time yourself as well.

---

Well, i never really learned any good ways to format stuff, do you know any tutorials / articles on how to format code?...Going to look around after this myself as well.

Quote:

---

Originally Posted by Salem

MinGW (actually GCC underneath) supports variable length arrays in C++ as an extension.

---

Ok, i didn't know that, but since the string length will always be a variable i think im going to continue using it for now. The other option would be to set it to a static size, although i dont like that because of the danger of a overflow since i dont know what will come in.

Quote:

---

The other option would be to set it to a static size, although i dont like that because of the danger of a overflow since i dont know what will come in.

---

The other option would be to set it to a dynamic size in a standard way.

Code:

---

char* argv = new char[(strlen(argz)+4)];
...
delete [] argv;

---

EDIT:
The code I posted above is C++. The C version would be:

Code:

---

char* argv = malloc(strlen(argz)+4);
...
free(argv);

---