5
\$\begingroup\$

For part of a project, I was given this directive:

The grep command will take an exact string and an exact filename as the 2 arguments and you will need to search through the filename for the given string and display the "line" and "line number" the string resides in. Example output would be:

# grep bob FileWithBob
12 and then bob picked up the phone and called jane 
24 even though they knew that bobby was not going to show up

Testing seems to give identical results to the actual grep program for everything I've tried:

$ ./my_grep "test" grep.c 
1:// test
2:// multi-line test

$ grep -n "test" grep.c 
1:// test
2:// multi-line test

grep.c:

// test
// multi-line test
#define _GNU_SOURCE 1       // for getline()
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <regex.h>
#include <unistd.h>
#include <sys/types.h>

#ifndef streq
#define streq(x, y) (strcmp((x), (y)) == 0)
#endif

#define MSGBUFSIZE  512

regex_t pattern;

int compile_pattern(const char *pat)
{
    int flags = REG_NOSUB;
    char error[MSGBUFSIZE] = "";

    int ret = regcomp(&pattern, pat, flags);
    if (ret)
    {
        regerror(ret, &pattern, error, sizeof(error));
        fprintf(stderr, "pattern `%s`: %s\n", pat, error);
        return ret;
    }

    return 0;
}

// read lines of text and match against the pattern
int process(FILE *fp)
{
    char *buf = NULL;
    size_t size = 0;
    char error[MSGBUFSIZE] = "";
    int ret = 0;

    for (int i = 1; getline(&buf, &size, fp) != -1; i++)
    {
        ret = regexec(&pattern, buf, 0, NULL, 0);
        if (ret)
        {
            if (ret != REG_NOMATCH)
            {
                regerror(ret, &pattern, error, sizeof(error));
                fprintf(stderr, "%s\n", error);
                free(buf);
                return ret;
            }
        }
        else
            printf("%d:%s", i, buf);
    }

    free(buf);
    return 0;
}

void usage(char* prog_name)
{
    fprintf(stderr, "usage: %s [-i] [-E] pattern [ files ... ]\n", prog_name);
    exit(1);
}

int main(int argc, char **argv)
{
    FILE *fp = NULL;

    if (1 == argc)
        usage(argv[0]);

    int err = compile_pattern(argv[optind]);
    if (err)
        return err;
    else
        optind++;

    if (optind == argc) // no files, default to stdin
        process(stdin);
    else {
        for (int i = optind; i < argc; i++)
        {
            if (streq(argv[i], "-"))
                process(stdin);
            else if ((fp = fopen(argv[i], "r")))
            {
                process(fp);
                fclose(fp);
            }
            else
            {
                fprintf(stderr, "%s: could not open: %s\n", argv[i], strerror(errno));
                err = -1;
            }
        }
    }

    regfree(&pattern);
    return err;
}

Any suggestions?

\$\endgroup\$
4
\$\begingroup\$

On the whole your code seems pretty concise and clean. I've just got a couple of side points.

Scope creep

This is a real problem in production code, where developers implement stuff that isn't really needed. Your specification you've posted says 'take an exact string and an exact filename'. You've allowed a regular expression to be passed. The original requirements could have been met by just using strstr which would have made the code quite a bit smaller. You've used extra time and added extra complexity, is there a real benefit?

The scope creep also means that I can't pass the basic string "\n" into your parser and have it find it as the '\n' is interpretted by the regex parser. So, with grep I can do:

grep -F "\n" someFile.h

And get lines with the text \n:

printf("some text\n");

Whereas running your code:

./a.exe "\n" someFile.h

I get every line that ends in a newline (i.e. every line of the file).

-h -i and arguments

If you're going to implement programs that take in arguments, it's a good idea to give the user some way to find out what the arguments mean. When I use grep -i, I'm trying to do a case-insensitive search. Your program advertises a '-i' option, however it doesn't do the same thing as grep. Instead I get told it can't open a file:

$ ./a.exe -i if someFile.h

if: could not open: No such file or directory

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.