0

I'm trying to write a simplified version of grep. It worked for most of the cases off the top of my head, but when I pass argument of "test" (ie "./grep test" in shell) and std input of

sajktestakjsdjka
jkskjfsdjfktestskjfkjsa
testsjhfhsjk'
sajkdjsatest

I get stdoutput of

sajktestakjsdjka
jkskjfsdjfktestskjfkjsa
testsjhfhsjk'
ts
sajkdjsatest

ts

Some characters appear to be UTF-8 and could not be copied to this post.

The only function I have that prints to stdout is

static void printnextline(char *buffer, char *str) {

    if (strstr(buffer, str) != NULL) {
        printf("%s\n", buffer);
    }

    free(buffer);
}

I'm guessing I have an issue with my implementation of the strstr function. I believe it should only return null when the string "test" is not within my buffer, but evidently its returning null in other cases as well. I presume I get weird lines of output because I'm printing a buffer pointing to random memory, but I don't understand why the strstr function is not returning null in such cases.

Maybe its a totally unrelated issue so here's the rest of the code

#include <stdlib.h>
#include <stdio.h>
#include <stdbool.h>
#include <string.h>

#define BASE_SIZE 5

bool errorexists(int argc) {
    if (argc != 2) {
        fprintf(stderr, "Please enter one and only one argument");
        return true;
    }
    return false;
}

static void printnextline(char *buffer, char *str) {

    if (strstr(buffer, str) != NULL) {
        printf("%s\n", buffer);
    }

    free(buffer);
}


static void checknextline(char *str) {

    char *buffer;
    buffer = malloc(BASE_SIZE * sizeof(char));
    int read = 0;
    int size_count = 1;
    char *backup;

    int c;
    while (((c = getchar()) != EOF) && (c != '\n')) {
        if (read == BASE_SIZE * size_count) {
            size_count++;
            backup = realloc(buffer, BASE_SIZE * size_count * sizeof(char));
            if (backup != NULL) {
                buffer = backup;
            } else {
                fprintf(stderr, "Ran out of memory to store line of characters");
                exit(EXIT_FAILURE);
            }
        }
        buffer[read] = (unsigned char) c;
        read++;
    }
    printnextline(buffer, str);
}


int main(int argc, char *argv[]) {

    if (errorexists(argc) == true) {
        exit(EXIT_FAILURE);
    }

    char *str = argv[1];

    while (!feof(stdin)) {
        checknextline(str);
    }

    exit(EXIT_SUCCESS);

}
user1953794
  • 27
  • 1
  • 7
  • Right off the bat - it looks like you're not null terminating `buffer`. – Michael Burr Jun 06 '15 at 06:45
  • 1
    ^^ yup - one of those 'you can debug it from the title' questions:) – Martin James Jun 06 '15 at 06:50
  • It's not a rule but not less than a rule, try to `free` the alloced memory in the same function where `malloc` is called. It will help you in maintenance of memory and it's a nicer way of coding. – Abhineet Jun 06 '15 at 06:55
  • string is nothing but just an array of characters + a null at last. The null for a string in C is same as the full-stop for a sentence in English. Now, when you see full-stop, you know, here you need to stop, the same way, `strstr` will see the null and will know where to stop, otherwise, it will try to keep on reading beyond the memory bounds and will end-up with some Undefined Behavior. – Abhineet Jun 06 '15 at 06:59
  • 2
    *while (!feof(stdin)) {* is incorrect. Read this: http://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong – chqrlie Jun 06 '15 at 08:02
  • 1
    allocating and freeing the buffer for each line seems inefficient. – chqrlie Jun 06 '15 at 08:03

1 Answers1

1

One reason is that you are not terminating buffer with '\0' in checknextline() function.

In that function you should do

while (((c = getchar()) != EOF) && (c != '\n')) {
        ... //your code
        buffer[read] = (unsigned char) c;
        read++;
    }
    buffer[read] = '\0';

Without this the strstr() function would not know where the string terminates and will access out of bound memory causing undefined behavior.

Rohan
  • 52,392
  • 12
  • 90
  • 87