patterncMinor
Search a file (simple grep clone)
Viewed 0 times
simplefilesearchgrepclone
Problem
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:
Testing seems to give identical results to the actual grep program for everything I've tried:
grep.c:
``
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 ... ]
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 upTesting 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 testgrep.c:
``
// test
// multi-line test
#define _GNU_SOURCE 1 // for getline()
#include
#include
#include
#include
#include
#include
#include
#include
#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 ... ]
Solution
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
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
$ ./a.exe -i if someFile.h
if: could not open: No such file or directory
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
Context
StackExchange Code Review Q#149583, answer score: 5
Revisions (0)
No revisions yet.