0

I've been trying to implement a search function in minix that will look for a file in the current or subdirectories and print the path. So far my code compiles without fail but it returns only a couple weird ascii characters for some reason, any idea of what I'm doing wrong?

#include <sys/types.h>
#include <sys/stat.h>
#include <dirent.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>

_PROTOTYPE(int main, (int argc, char **argv));
_PROTOTYPE(char *find_file, (char *name, char *directory));

int main(argc, argv)
int argc;
char **argv;
{
    char *path = find_file(argv[0], "./");
    if (strcmp(path, "") == 0) {
        printf("file could not be found.");
    } else {
        printf("File found in: %s\n", path);
    }
    return(0);
}

char *find_file(name, directory)
char *name;
char *directory;
{
    DIR *d;
    struct dirent *e;
    struct stat s;

    char *dr;
    char *res;
    char *result;

    d = opendir(directory);
    if (d != NULL) {
        while (e = readdir(d)) {
            if (e->d_name == name)
                return name;
        }
    }
    closedir(d);

    d = opendir(directory);
    if (d != NULL) {
        while (e = readdir(d)) {
            stat(e->d_name, &s);
            if (s.st_mode & S_IFDIR) {
                dr = malloc(strlen(directory) + strlen(e->d_name) + 2);
                strcpy(dr, directory);
                strcat(dr, e->d_name);
                strcat(dr, "/");
                res = find_file(name, dr));
                if (strcmp(res, "") != 0) {
                    strcpy(result, e->d_name);
                    strcat(result, "/");
                    strcat(result, res);
                    return result;
                }
            }
        }
    }
    closedir(d);
    return "";
}

So I first check if the file is in the current directory before going into the child folders that's why I open and close the directory twice. The only thing I suspect may be unorthodox is using malloc straight off the bat and declaring a set amount, is that a no no? Thanks for the help <3

EDIT: so I tried to use malloc with the size of the string instead of the set amount but no change, here's a screenshot:

enter image description here

EDIT2: Updated my code thanks to the suggestions, still not working 100% as it's going parent folders or something weird like that, will post the solution if I manage to get it working perfectly (y)

EDIT3: I've managed to get it working (to some degree) it can work perfectly but in some cases it doesn't find the existing file, don't know the reasons for this and too tired to determine why ^_^ here's the final working code for anyone else who will look for a similar solution in the future:

#include <sys/types.h>
#include <sys/stat.h>
#include <dirent.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>

_PROTOTYPE(int main, (int argc, char **argv));
_PROTOTYPE(char *find_file, (char *name, char *directory));

int main(argc, argv)
int argc;
char **argv;
{
    char *path = find_file(argv[1], "./");
    if (strcmp(path, "") == 0) {
        printf("file could not be found.\n");
    } else {
        printf("File found in: %s\n", path);
    }
    return(0);
}

char *find_file(name, directory)
char *name;
char *directory;
{
    DIR *d;
    struct dirent *e;
    struct stat s;

    char *dr;
    char *res;
    char *result;

    d = opendir(directory);
    if (d != NULL) {
        while (e = readdir(d)) {
            if (strcmp(e->d_name, name) == 0)
                return e->d_name;
        }
    }
    closedir(d);

    d = opendir(directory);
    if (d != NULL) {
        while (e = readdir(d)) {
            stat(e->d_name, &s);
            if (strcmp(e->d_name, ".") != 0 && strcmp(e->d_name, "..") != 0) {    
                if (s.st_mode & S_IFDIR) != 0) {
                    dr = malloc(strlen(directory) + strlen(e->d_name) + 2);
                    strcpy(dr, directory);
                    strcat(dr, e->d_name);
                    strcat(dr, "/");
                    res = find_file(name, dr));
                    if (strcmp(res, "") != 0) {
                        result = malloc(strlen(e->d_name) + strlen(res) + 2);
                        strcpy(result, e->d_name);
                        strcat(result, "/");
                        strcat(result, res);
                        return result;
                    }
                }
            }
        }
    }
    closedir(d);
    return "";
}

For some reason the file name was passed in argv1 not argv[0] which is weird since I've implemented another function that passed the file name through argv[0]... Minix ¯|(ツ)

Cœur
  • 37,241
  • 25
  • 195
  • 267
Shockwave
  • 27
  • 5
  • TL;DR. `path = find_file(argv[0], "./");` is a bad sign - you would seem to be overwriting your pointer to malloced memory with something else (causing a leak). Actually no frees anywhere, so it's pretty leaky. – John3136 Nov 09 '15 at 08:58
  • Any suggested changes? I did try taking out the malloc and initializing path to the find_file result but it still outputs garbage :<. If I free the variables apart from result before the return will that help? – Shockwave Nov 09 '15 at 09:04
  • The code would not compile on my machine. Did you posted it correctly? For example there is a missing semicolon after struct stat and other errors. Also if (res != "") ---> if(res[0] != '\0') or use strcmp. – terence hill Nov 09 '15 at 10:09
  • Thanks, I saw that mistake after you pointed it out, corrected it now :) – Shockwave Nov 09 '15 at 11:31
  • the C code layout is from back in the days when Richie and C. were first developing the language. Suggest getting a modern compiler. Note: gcc is a free version with all the latest 'whiz bangs`. – user3629249 Nov 11 '15 at 03:05
  • when calling the memory allocation functions: `malloc()`, `calloc()` and `realloc()` always check (!=NULL) the returned value to assure the operation was successful. – user3629249 Nov 11 '15 at 03:19
  • regarding this line: char *path = find_file(argv[0], "./");, argv[0] is always the name of the currently running program. Suggest using: char *path = find_file(argv[1], "./"); – user3629249 Nov 11 '15 at 04:01
  • this line: `char *result;` fails to allocate space for the result string. so the lines like: `strcpy(result, e->d_name);` are writing to what ever trash address is currently on the stack as the location of `result`. This is undefined behaviour and can/will lead to a seg fault event. – user3629249 Nov 11 '15 at 04:04
  • There are several calls to `malloc()` in the posted code, but no matching calls to `free()`, so there are many memory leaks, one for each call to malloc – user3629249 Nov 11 '15 at 04:07
  • the posted code does not step forward through the dir entries and appends the initial 'path+file name' over and over and over. Suggest spending a bit of time actually debugging the code AND search stackoverflow.com for examples of stepping through directorys. See my answer for your code, modified so it actually compiles. and the resulting output – user3629249 Nov 11 '15 at 04:12

3 Answers3

0

You are giving an unintialized pointer to stat.

struct stat *s
....
stat(e->d_name, s);

What you should do instead is:

struct stat s
....
stat(e->d_name, &s);

Another problem (among other things) is this part:

strcpy(res, find_file(name, dr));
if (res != "") {

You can not compare string like this in C, you have to use strcmp(res, "") == 0, because you are comparing pointers here. It may even work in this case (if static strings are reused by your compiler), but in the general case it will not.

Devolus
  • 21,661
  • 13
  • 66
  • 113
  • Ok will try it out now (y) – Shockwave Nov 09 '15 at 11:00
  • It's not spitting out garbage now! its not printing out correct input but I think that's something I need to fix on my end. Will mark your answer as correct as it did solve most of my problem :P – Shockwave Nov 09 '15 at 11:08
0

I am not familiar with Minix but you may want to exclude '.' and '..' directories from your search since Minix is also a UNIX-like system.

Secondly, you don't have to open a directory twice because readdir() will walk through all the items under a directory, including '.' and '..', whether it is a directory or file.

Thirdly, for this code block:

d = opendir(directory);
if (d != NULL) {
    while (e = readdir(d)) {
        if (e->d_name == name)
            return name;
    }
}
closedir(d);

It is not very good to just return name here because it is only a file name without information about its absolute path. That's also why you should incorporate this code block into the second part of your find_file().

Also, you should not return the result here because it is very probable that there is another item under another directory with the same name you are searching for. A better solution would be just print out the name (with the absolute path info) and continue with the process. Or simply put, you don't have to make your find_file() to return something. You only have to print out the name you already find.

0
#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h> // added
#include <dirent.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>


char *find_file(char *name, char *directory); // corrected

int main(int argc, char * argv[]) // corrected
{
    // added to correct compiler warning about unused parameter 'argc'
    if( 2 != argc ) {perror( "wrong number of parameters "); exit( EXIT_FAILURE); }

    // corrected to use parameter rather than current executable name
    char *path = find_file(argv[1], "./");

    // corrected to make useful comparison
    if ( '\0' == path[0] )
    {
        printf("file could not be found.");
    }

    else
    {
        printf("File found in: %s\n", path);
    }

    return(0);
} // end function: main

char *find_file(char *name, char *directory) // corrected
{
    DIR *d;
    struct dirent *e;
    struct stat s;

    char *dr;
    char *res;
    char *result = malloc( 1024 ); // corrected so 'result' pointer to allocated memory

    printf("initial DIR: %s\n", directory); // added for debug

    d = opendir(directory);
    if (d != NULL)
    {
        // corrected to fix compiler warning
        while ( NULL != (e = readdir(d)) )
        {
            if (e->d_name == name)
                return name;
        }
    }
    closedir(d);

    d = opendir(directory);
    if (d != NULL)
    {
        // corrected to fix compiler warning
        while ( NULL != (e = readdir(d)) )
        {
            stat(e->d_name, &s); // corrected
            // corrected so actually works
            if (S_ISDIR(s.st_mode) )
            {
                dr = malloc(strlen(directory) + strlen(e->d_name) + 2);
                strcpy(dr, directory);
                strcat(dr, e->d_name);
                strcat(dr, "/");

                // added for debug
                printf( "nested DIR: %s\n", dr);

                res = find_file(name, dr);

                // corrected to make useful comparison
                if ( '\0' != res[0] )
                {
                    strcpy(result, e->d_name);
                    strcat(result, "/");
                    strcat(result, res);
                    return result;
                } // end if
            } // end of
        } // end while
    } // end if
    closedir(d);
    return "";
} // end function: find_file

(Note: this leaks memory at every iteration as the malloc'd memory is not being passed to free()

which gives the following output:

(Note: this is a snapshot of one repitition of a repetitive output after the code has run for a few seconds.)

(Note: helloworld is an executable file, in the middle (alphabetically) of a directory of some 70 entries)

(Note: the command line was: ./untitled untitled.c

initial DIR: ./helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/../helloworld/

user3629249
  • 16,402
  • 1
  • 16
  • 17