free() usage

I'm having a problem with a loop in my code. Basically what I'm trying to do is to have a "/" and the filename concatenated to the end of a directory path that the user provides at the command line. So for example the user inputs "/usr" then when the program scans through each entry in this particular directory, it should be able to get the full physical path such as "/usr/file1". Instead what happens is the following.

/usr/.//
/usr/./..//
/usr/./file1///
/usr/./../file1/file2////

Of course this is not what I want, I know why this is happening because it's thanks to this little piece of code.

Code:

---

if(S_ISDIR(sbuf.st_mode)){ /* Is a directory */
        count = scandir(argv[1],&direntp,NULL,alphasort);
        if(count){
          for(index=0; index<count; ++index){
            printDetails(direntp[index]->d_name,argv[1]);
            free(direntp[index]);
          }
        }else{
          fprintf(stderr,"Can't open %s\n",argv[1]);
          exit(EXIT_FAILURE);
        }
}

---

Every time it encounters an entry it loops through and passes both the entry name (direntp[index]->d_name) and the path that has been provided (argv[1]). Also this code in the printDetails function plays a part to the bug.

Code:

---

}else{
    strcat(path,"/");
    strcat(path,filename);
    fprintf(stderr,"Path: %s\n",path);
    fprintf(stderr,"Can't stat %s\n",curdir);
    return;
  }

---

Now the thing is i've also done the concatenating before with full success from the following code.

Code:

---

/*Concatenate dir names*/
  curdir = workdir();
  strcat(curdir,"/");
  strcat(curdir,filename);

  /*stat the final dir name*/
  if(stat(curdir,&sbuf) != -1){
    if(S_ISLNK(sbuf.st_mode)){
      if(readlink(curdir,buf,bufsize) != -1){
        if(lstat(buf,&sbuf) == -1){
          fprintf(stderr,"Can't stat %s\n",filename);
          return;
        }
      }else
        fprintf(stderr,"Can't readlink %s\n",filename);
    }else if(S_ISREG(sbuf.st_mode)){
      if((fdes = open(curdir,O_RDONLY)) != ENOENT){
        if(fstat(fdes,&sbuf) == -1){
          fprintf(stderr,"Can't stat %s\n",filename);
          return;
        }
        close(fdes);
      }else{
        fprintf(stderr,"Can't open %s\n",filename);
        return;
      }
    }
  }else{
    // previously listed code
  }

---

And just to make sure i haven't missed anything heres all the code. In case you haven't guessed the following code should perfrom something similar to the "ls -al" unix command.

Code:

---

#include <stdio.h>
#include <sys/types.h>
#include <fcntl.h>
#include <dirent.h>
#include <stdlib.h>
#include <time.h>
#include <errno.h>
#include <string.h>
#include <sys/stat.h>
#include <pwd.h>
#include <grp.h>
#include <unistd.h>

#define SIZE 256

/*prototypes*/
char* workdir(void);
void printDetails(char* filename,char* path);
int main(int argc,char** argv);

char* workdir(){
  size_t size = FILENAME_MAX;
  while(1){
    char* wdbuf = (char*)malloc(size);
    if(getcwd(wdbuf,size) == wdbuf)
      return wdbuf;
    free(wdbuf);
    if(errno != ERANGE){
      fprintf(stderr,"Out of range\n");
      return 0;
    }
    size *=2;
  }
}

void printDetails(char *filename,char *path){

  struct passwd *my_passwd;
  struct group *my_group;
  struct stat sbuf;
  struct tm *strtime;

  int fdes;
  unsigned char type ='?';
  unsigned char usr[] = "---";
  unsigned char grp[] = "---";
  unsigned char oth[] = "---";
  unsigned char uid[9], gid[9];
  unsigned char mod[13];
  mode_t mode;
  char* curdir;
  char* buf;
  size_t bufsize =FILENAME_MAX;
  time_t now;
  int year;

  /*Concatenate dir names*/
  curdir = workdir();
  strcat(curdir,"/");
  strcat(curdir,filename);

  /*Fails because it uses the incorrect
    directory path. Eg. if /root was
    passed as the argument then sub dirs
    within root would try to be stat'd with
    for example /root/ls/temp instead of
    /root/temp . */

  /*stat the final dir name*/
  if(stat(curdir,&sbuf) != -1){
    if(S_ISLNK(sbuf.st_mode)){
      if(readlink(curdir,buf,bufsize) != -1){
        if(lstat(buf,&sbuf) == -1){
          fprintf(stderr,"Can't stat %s\n",filename);
          return;
        }
      }else
        fprintf(stderr,"Can't readlink %s\n",filename);
    }else if(S_ISREG(sbuf.st_mode)){
      if((fdes = open(curdir,O_RDONLY)) != ENOENT){
        if(fstat(fdes,&sbuf) == -1){
          fprintf(stderr,"Can't stat %s\n",filename);
          return;
        }
        close(fdes);
      }else{
        fprintf(stderr,"Can't open %s\n",filename);
        return;
      }
    }
  }else{
    strcat(path,"/");
    strcat(path,filename);
    fprintf(stderr,"Path: %s\n",path);
    fprintf(stderr,"Can't stat %s\n",curdir);
    return;
  }     

  /*format information*/
  mode = sbuf.st_mode;
  if( S_ISLNK( mode ) ) type = 'l'; /*symlink*/
  else if( S_ISDIR ( mode ) ) type = 'd'; /*directory*/
  else if( S_ISCHR ( mode ) ) type = 'c'; /*character raw device*/
  else if( S_ISBLK ( mode ) ) type = 'b'; /*block raw device*/
  else if( S_ISFIFO( mode ) ) type = 'p'; /*named pipe*/
  else if( S_ISSOCK( mode ) ) type = 's'; /*Unix domain socket*/
  else if( S_ISREG ( mode ) ) type = '-'; /*regular file*/
  if( mode & S_IRUSR ) usr[0] = 'r';
  if( mode & S_IWUSR ) usr[1] = 'w';
  if( mode & S_IXUSR ) usr[2] = 'x';
  if( mode & S_ISUID ) usr[2] = 's'; /* set UID bit*/
  if( mode & S_IRGRP ) grp[0] = 'r';
  if( mode & S_IWGRP ) grp[1] = 'w';
  if( mode & S_IXGRP ) grp[2] = 'x';
  if( mode & S_ISGID ) grp[2] = 's'; /* set GID bit*/
  if( mode & S_IROTH ) oth[0] = 'r';
  if( mode & S_IWOTH ) oth[1] = 'w';
  if( mode & S_IXOTH ) oth[2] = 'x';

  my_passwd = getpwuid(sbuf.st_uid);
  if(my_passwd)
    strncpy(uid,my_passwd->pw_name,8);
  else
    snprintf(uid,9,"%d",sbuf.st_uid);

  my_group = getgrgid(sbuf.st_gid);
  if(my_group)
    strncpy(gid,my_group->gr_name,8);
  else
    snprintf(gid,9,"%d",sbuf.st_gid);

  time(&now);
  year = localtime(&now)->tm_year;
  strtime = localtime(&sbuf.st_ctime);
  if(strtime->tm_year == year)
    strftime(mod,13,"%b %e %R",strtime);
  else
    strftime(mod,13,"%b %e %Y",strtime);
       
  /*print the details*/
  printf("%c%s%s%s    %d %s    %s        %d\t%s %s\n",
        type,usr,grp,oth,sbuf.st_nlink,
        uid,gid,sbuf.st_size,mod,filename);
  free(curdir);
  /*free(path);*/
}

int main(int argc, char **argv) {
  int count,index;
  char *curdir;
  struct stat sbuf;
  struct dirent **direntp;

  curdir = workdir();

  if(argv[1] != '\0'){
    if(stat(argv[1], &sbuf) != -1){
      if(S_ISDIR(sbuf.st_mode)){ /* Is a directory */
        count = scandir(argv[1],&direntp,NULL,alphasort);
        if(count){
          for(index=0; index<count; ++index){
            printDetails(direntp[index]->d_name,argv[1]);
            free(direntp[index]);
          }
        }else{
          fprintf(stderr,"Can't open %s\n",argv[1]);
          exit(EXIT_FAILURE);
        }
      }else if(S_ISREG(sbuf.st_mode)){ /* Is a regular file */
        printDetails(argv[1],NULL);
      }else if(S_ISLNK(sbuf.st_mode))
        printDetails(argv[1],NULL);             
    }else
      printf("Can't stat %s\n", argv[1]);
  }else{
    printf("%s\n",curdir);
    if(stat(curdir,&sbuf) !=-1){
      count = scandir(curdir,&direntp,NULL,alphasort);
      if(count){
        for(index=0; index<count; ++index){
          printDetails(direntp[index]->d_name,NULL);
          free(direntp[index]);
        }
      }else{
        fprintf(stderr,"Can't open %s\n",curdir);
        exit(EXIT_FAILURE);
      }
    }else
      fprintf(stderr,"Can't stat %s\n",curdir);
  }
  free(curdir);
  free(direntp);
  return 0;
}

---

I thought about trying

Code:

---

free(path);

---

But this doesn't seem to work.

Ok, I'll give you three guesses to tell me what is wrong with this chain of events:

1) void printDetails(char *filename,char *path){
... the prototype

2) printDetails(argv[1],NULL);
... the call

3) strcat(path,"/");
... the strcat


Gee. Any idea why that fails?
Additionally, you can't free a null pointer. Well you CAN...

Quzah.

I'd agree with that diagnosis, except that printDetails only gets executed when the command line argument it encounters is either a link or a regular file.

Code:

---

}else if(S_ISREG(sbuf.st_mode)){ /* Is a regular file */
        printDetails(argv[1],NULL);
}else if(S_ISLNK(sbuf.st_mode))
        printDetails(argv[1],NULL);

---

What i'm talking about is when the command line encounters a directory.

Code:

---

if(S_ISDIR(sbuf.st_mode)){ /* Is a directory */
        count = scandir(argv[1],&direntp,NULL,alphasort);
        if(count){
          for(index=0; index<count; ++index){
            printDetails(direntp[index]->d_name,argv[1]);
            free(direntp[index]);
          }
        }else{
          fprintf(stderr,"Can't open %s\n",argv[1]);
          exit(EXIT_FAILURE);
        }
}

---

And besides the concatenation code should only be reached if the following code fails.

Code:

---

  if(stat(curdir,&sbuf) != -1){
    if(S_ISLNK(sbuf.st_mode)){
      if(readlink(curdir,buf,bufsize) != -1){
        if(lstat(buf,&sbuf) == -1){
          fprintf(stderr,"Can't stat %s\n",filename);
          return;
        }
      }else
        fprintf(stderr,"Can't readlink %s\n",filename);
    }else if(S_ISREG(sbuf.st_mode)){
      if((fdes = open(curdir,O_RDONLY)) != ENOENT){
        if(fstat(fdes,&sbuf) == -1){
          fprintf(stderr,"Can't stat %s\n",filename);
          return;
        }
        close(fdes);
      }else{
        fprintf(stderr,"Can't open %s\n",filename);
        return;
      }
    }
  }

---

My point is still relevant. Additionally, you cannot simply concat onto the end of the argv pointers. This is like trying to:

char *s = "Hello. Concatting here is a BadThing(TM)";

strcan( s, "This is wrong to do." );

While your compiler MAY let you do it, it is WRONG to do so. Command line arguments, if I recall correctly, are just like string literals. They are not to be modified. This is your problem.

Besides...

Quote:

---

And besides the concatenation code should only be reached if the following code fails.

---

Even if it "should" only ever bee reached in some rare occasion, my point is still valid:

THIS WILL CRASH YOUR PROGRAM:

Quote:

---

}else if(S_ISREG(sbuf.st_mode)){ /* Is a regular file */
printDetails(argv[1],NULL);
}else if(S_ISLNK(sbuf.st_mode))
printDetails(argv[1],NULL);
}else
printf("Can't stat %s\n", argv[1]);
}else{

---

Concating onto a NULL pointer WILL crash your program. As such, you should fix this, because it is potentially erronious code.

And like I said: You cannot concat onto string literals.

Quzah.

Quote:

---

char *s = "Hello. Concatting here is a BadThing(TM)";

strcan( s, "This is wrong to do." );

---

Why is it wrong tho??

>Why is it wrong tho??
You cannot modify a string literal.

-Prelude