[Okta Webinar] Learn how to a build a cloud-first strategyRegister Now

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 410
  • Last Modified:

Pls help with tihs code

Okay, as some folks may know, I am learning C and the following is an assignment I just submitted. My task is to write a l s like program in Unix. It took me sometime.

Required features are to implement -i -l argument, and take care of absolute & relative path issues. Optional feature is to implement -R.

I got lost in relative path & - R features. I would appreciate your time so much if you could take a look at my code, and give me some suggestions.

I have to admit C is hard to learn, esp pointers. Hopefully I can make it later. Weird enough, when I looked back, I think what I have done is so easy, but I did not think so a few days ago.

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

/*

Get inode/name pairs of the directory.
If a name/inode pair says it is a regular file, print out its information according to option provided
If a name/inode pair indicates it is a directory or link file, then use different policies.
In either case, the option as a parameter will be transferred.

*/
int option; //Global;Use to tell -R -l -i arguments
void filestat(struct dirent *); //print file information
void dirLoop(char *); //traverse the directory

void dirLoop(char *name)
 {
    DIR *dir; //directory
    char *fileName;
    struct dirent *pair; // name & inode pair
    //chdir(*name);
    //chdir(*name);
    dir=opendir(name);
    //chdir(dir);
    if (dir !=NULL)
   {
      while (pair=readdir(dir))
         {
          chdir(dir);
          //ingore "." and ".."
            if (strcmp(pair->d_name,".")==0 ||
                strcmp(pair->d_name,"..")==0)
                  continue;
            else
               filestat(pair);
         }
       close(dir);
   }
   else
   {
      fprintf(stderr,"Open directory error: %s\n",name);
      close(dir);
      return;
   }
    return;
 }
 
/*
 Print out file information
*/

void filestat(struct dirent *entry)
{
   
   struct permission {
      char dl;
      char owner_read;
      char owner_write;
      char owner_exe;
      char grp_read;
      char grp_write;
      char grp_exe;
      char other_read;
      char other_write;
      char other_exe;
   };
   
   struct permission modes;
   struct passwd  *pwd;
   struct group   *grp;
   struct tm *sometime;
   char timeString[18];
   char star,star2;
   struct stat filebuff;
   lstat(entry->d_name,&filebuff);
   //chdir(entry->d_name);

   
   if (strncmp(entry->d_name,".",1)==0) return;//Do not include those starting with .
 
   //For printing time later
   sometime=localtime(&filebuff.st_mtime);
   
   if ((filebuff.st_mode & S_IFDIR)==S_IFDIR)//first char of the 10 access mode chars
   modes.dl='d';
   else
   if ((filebuff.st_mode & S_IFLNK)==S_IFLNK)
   modes.dl='l';
   else
   modes.dl='-';

   //Values for drwxrwxrwx

   modes.owner_read=      (filebuff.st_mode & S_IRUSR)==S_IRUSR ? 'r':'-';
   modes.owner_write=      (filebuff.st_mode & S_IWUSR)==S_IWUSR ? 'w':'-';
   modes.owner_exe=      (filebuff.st_mode & S_IXUSR)==S_IXUSR ? 'x':'-';
   modes.grp_read=      (filebuff.st_mode & S_IRGRP)==S_IXGRP ? 'r':'-';
   modes.grp_write=      (filebuff.st_mode & S_IWGRP)==S_IXGRP ? 'w':'-';
   modes.grp_exe=      (filebuff.st_mode & S_IXGRP)==S_IXGRP ? 'x':'-';
   modes.other_read=      (filebuff.st_mode & S_IROTH)==S_IROTH ? 'r':'-';
   modes.other_write=      (filebuff.st_mode & S_IWOTH)==S_IWOTH ? 'x':'-';
   modes.other_exe=       (filebuff.st_mode & S_IXOTH)==S_IXOTH ? 'x':'-';
   //the char after file name, *,/,@
   star= ((filebuff.st_mode & S_IXUSR) ||
   (filebuff.st_mode & S_IXGRP) ||
   (filebuff.st_mode & S_IXOTH))
   ? '*' : ' ';
   if (S_ISLNK(filebuff.st_mode)) star='@';
   if (S_ISDIR(filebuff.st_mode)) star='/';
   if (S_ISLNK(filebuff.st_mode))
   star2=' ';
   else
   star2=star;
   
   int count;
   count=0;

   //no arguments provided
   if (option==0)
   {
      count++;
      printf("%s",entry->d_name);
      if (S_ISDIR(filebuff.st_mode))
      printf("/");
      else if (S_ISLNK(filebuff.st_mode))
      printf("@");
      else if ((filebuff.st_mode & S_IXUSR) ||
       (filebuff.st_mode & S_IXGRP) ||
          (filebuff.st_mode & S_IXOTH) && !S_ISLNK(filebuff.st_mode) && !S_ISLNK(filebuff.st_mode))
      printf("*");
      printf("%4s"," ");
      if ((count%4)==0) printf("\n");
      return;
   }
   
   
   //print Inode
   if (option==10 || option==11 || option==110 || option==111)
   {
      count++;
      printf("%12u\t",filebuff.st_ino);
      if (option<100)
      {
       printf("%-s%c\n",entry->d_name,star);
       //if ((count%4)==0) printf("\f");
       return;
      }
     
   }
   
   if (option>11)
   {
   //print access mode
   printf("%.10s",modes);
   
   //print # of hard links
   printf(" %3u ",filebuff.st_nlink);
   
   //print user name and group name
   if ((pwd = getpwuid(filebuff.st_uid)) != NULL)
          printf(" %-8.8s", pwd->pw_name);
   else
          printf(" %-8d", filebuff.st_uid);
   
   if ((grp = getgrgid(filebuff.st_gid)))
          printf(" %-10.10s", grp->gr_name);
   else
          printf(" %-10d", filebuff.st_gid);
   
   printf("%8ld ",filebuff.st_size);
          
   strftime( timeString, 25 , "%b %d %Y %H:%M ", sometime);
          
   printf(" %s ",timeString);
   }
   printf("%s",entry->d_name);
   printf("%c",star2);
   
   if (S_ISLNK(filebuff.st_mode))
   {
      char buf[128];
      readlink(entry->d_name, buf, 512);
      printf("-> %s", buf);
   }
   
   printf("\n");

   //-R

      if (S_ISDIR(filebuff.st_mode) && (option % 2)!=0)
      {

      }

   return;

}

int main (int argc, char* argv[])
{
    char* dirName;
    int index,i,l,R;
    i=0;l=0;R=0;
   
      if (argc==1) //command only
            dirName=".";
    else
            if (argv[argc-1][0]=='-')
        {
                  dirName=".";
/*
Step 1: Get directory's name and Identity valid option
*/
         for (index=1;index<argc;index++)
            {
                if (strchr(argv[index],'l')!=NULL)
                l=100;
                if (strchr(argv[index],'i')!=NULL)
                i=10;
                if (strchr(argv[index],'R')!=NULL)
                R=1;  
            }
        }
        else
        {
            dirName=argv[argc-1];
          
            for (index=1;index<argc-1;index++)
            {
                if (strchr(argv[index],'l')!=NULL)
                l=100;
                if (strchr(argv[index],'i')!=NULL)
                i=10;
                if (strchr(argv[index],'R')!=NULL)
                R=1;      
            }
        }
       
/*
Step 2: Caculate option
*/
        option=l+i+R;  
/*
Step 3: Step into the directory
*/
            dirLoop(dirName);

   printf("\n");

    return 0;
}
 
0
tiger0516
Asked:
tiger0516
  • 2
1 Solution
 
fridomCommented:
magic numbers are a really bad idea. So you should have used some integer switches and do the option parsing a bit more C-ish, but ok, you can do it that ways.

But your option parsing is still broken what happens if you have
./my_prog some_file_with_an_i_init; or an l or someother thing. You simply parse the args with strstr so you are not really dispatching on optons but on letters. That's broken

However there is at least one real problem in you code you treat:
 struct permission modes; simply as a "String"
printf("%.10s",modes);

but it is in reallity a structure, Maybe you got have luck and got some output but this surely is wrong.

You have some copy+ paste errors in you code also:
modes.grp_read=     (filebuff.st_mode & S_IRGRP)==S_IXGRP ? 'r':'-';
   modes.grp_write=     (filebuff.st_mode & S_IWGRP)==S_IXGRP ? 'w':'-';
   modes.grp_exe=     (filebuff.st_mode & S_IXGRP)==S_IXGRP ? 'x':'-';

This is clearly wrong.

For my taste you did way to much in one function.

Your indenting makes it difficult to follow the control flow (that is of course just important for readers, the computer feels fine about no indentation at all.

And in one part you are using a pointer you misuse it:
char* dirName;
    int index,i,l,R;
    i=0;l=0;R=0;
   
     if (argc==1) //command only
          dirName=".";
    else
          if (argv[argc-1][0]=='-')
        {
               dirName=".";

You have just  pointer with no initialzed data and you are assing it a string containing at least two elements the . and the trailing zero. you should have written:
char dirName[SOME_LIMIT];

and
strcpy(dirName, ".");

Your compiler surely has warned you about this...

You can provide options at any place but you are fixing a few things. You can do that but it would be better to not do that.

So the outcome will not all to well I guess, but at least it's  a fair try. You errors happen if you do not have experience

You do not understand currently what  char *some_val stands for. This will be  a major drawback in you futher undertakings. I would really work hard to understand this stuff.


This is just what I've seen on first sight. But that should be enough to keep you "working" ;-)

Regards
Friedrich
0
 
fridomCommented:
Sorry I made mistake the dirName = assignment is of course ok. I don't know why I did that wrong.

Regards
Friedrich
0
 
brettmjohnsonCommented:
>       close(dir);

You probably want to call closedir(dir), rather than close().


0

Featured Post

Industry Leaders: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

  • 2
Tackle projects and never again get stuck behind a technical roadblock.
Join Now