4
\$\begingroup\$

This is my implementation of logfind from Learning C The Hard Way By Zed Shaw.

Please let me what you think and how I can improve and optimize this implementation.

Also, please tell me what I did right so that I keep doing it. Thank you for your feedback!

Example:

I want a tool called logfind that lets me search through log files for text. This tool is a specialized version of another tool called grep, but designed only for log files on a system. The idea is that I can type: logfind zedshaw And, it will search all the common places that log files are stored, and print out every file that has the word “zedshaw” in it.

Shaw, Zed A. (2015-08-10). Learn C the Hard Way: Practical Exercises on the Computational Subjects You Keep Avoiding (Like C) (Zed Shaw's Hard Way Series) (Kindle Locations 5628-5632). Pearson Education. Kindle Edition.

logfind.c

    /* 
 * MAC OSX Log Finder
 * ------------------
 * usage: logfind [-o] target1 [target2, ...]  
 *  target1, target2, etc.: the strings to search
 *  -o: evaluates targets with a logical or (default is logical and)
 */

#include <stdlib.h>
#include <stdio.h>
#include <unistd.h> 
#include <string.h> 
#include <glob.h> 
#include "../debug/dbg.h" 


#define USAGE \
        "\n\nusage: logfind [-o] target1 [target2, ...]\
        \n\n\ttarget1, target2, etc.: the strings to search\n\
        -o: evaluates targets with a logical or (default is logical and)\n" 

#define HELP \
        "\nMAC OSX Log Finder\nCreated By: Raisel Martinez\n"

enum Logic {
    AND, OR 
};

static char logic = AND;
static int patterns = 0; 
static size_t tcont_size = 0; 

int scpyalloc(char *strings[], char *src, int index)
{
    size_t strsize = strlen(src);

    strings[index] = (char*) malloc(strsize + 1);
    check_mem(strings[index]);

    strncpy(strings[index], src, strsize);
    strings[index][strsize] = '\0';

    check(*strings[index] == *src, "Copy Error"); 

    return 0;
error:
    return 1;   
}

int parse_args(char* targets[], char* argv[], int argc)
{
    int i = 1;
    int t = 0;
    int rc = 0;

    for (i = 1; i < argc; i++) {
        char *current = argv[i]; 
        if (current[0] == '-') {
            switch (current[1]) {
                case 'o':
                    logic = OR; 
                    break;
                case 'h':
                    printf(HELP);
                    goto help; 

                default:
                    log_err(USAGE);
                    goto error;     
            }   
        } else {
            rc = scpyalloc(targets, current, t);
            check(rc == 0, "Allocation Error"); 
            ++t;
            ++tcont_size; 
        }   
    }

    return 0; 

error:
    return 1;

help:
    return 2;   
}

int ldpatterns(char** fpatterns)
{
    int index  = 0;
    size_t len = 0;
    char *line = NULL; 
    ssize_t read;

    FILE *dirfile = fopen(".logfind", "r");
    check_mem(dirfile);

    while ((read = getline(&line, &len, dirfile)) != -1) {
        fpatterns[index] = (char*) malloc(len);
        check_mem(fpatterns[index]); 

        strncpy(fpatterns[index], line, len);   

        fpatterns[index][strcspn(fpatterns[index], "\n")] = 0; 

        ++index;
        ++patterns;     
    }

    return 0;

error:
    return 1;

}

char* concat(const char *a, const char *b)
{
    size_t len_a = strlen(a); 
    size_t len_b = strlen(b); 

    char *result = (char*) malloc(len_a + len_b + 1); 
    check_mem(result); 

    memcpy(result, a, len_a); 
    memcpy(result + len_a, b, len_b + 1);

    return result; 

error:
    return NULL; 
}

char* ftobuff(FILE *in)
{
    char *buff = NULL;  
    size_t bufsize = 0;

    check_mem(in);

    if (fseek(in, 0L, SEEK_END) == 0) {

        bufsize = ftell(in); 
        check((int)bufsize != -1, "File Buffer Error"); 

        buff = (char*)malloc(bufsize + 1);

        check(fseek(in, 0L, SEEK_SET) == 0, "File Seek Error"); 

        size_t nlen = fread(buff, 1, bufsize, in);
        check(ferror(in) == 0, "File Read Error");

        buff[nlen++] = '\0';
    }       

    return buff; 

error:
    return NULL; 
}

int strfmatch(FILE *file, char *target)
{
    int match = 1;  
    char *tokbuf = NULL; 
    char *fbuff = ftobuff(file);  
    char delim[] = " ,:/\t\r\n\v\f";

    check_mem(fbuff);

    tokbuf = strtok(fbuff, delim);

        while (tokbuf) {
        if ((strcmp(target, tokbuf)) == 0) {
            match = 0; 
        }

        tokbuf = strtok(NULL, delim); 
    }

    return match;   

error:
    return -1;  
}

int and_match(char **targets, FILE *comp)
{
    size_t i = 0;
    int match = 0; 

    check_mem(targets)
    check_mem(comp);

    for (i = 0; i < tcont_size; i++) {
        if (strfmatch(comp, targets[i]) == 0) {
            ++match; 
        }
    }

    if (match == (int)tcont_size) {
        return 0;
    } else {
        return 1; 
    }

error:
    return -1; 
}

int or_match(char **targets, FILE *comp)
{   
    size_t i = 0;
    int ret = 1;

    check_mem(targets); 
    check_mem(comp);

    for (i = 0; i < tcont_size; i++) {
        if (strfmatch(comp, targets[i]) == 0) {
            ret = 0;
            break; 
        }       
    }   

    return ret; 

error:
    return -1; 
}   


int process_results(char **targets, char **globbuf, size_t bufsize)
{   
    size_t i = 0;
    int rc = 0;

    check(bufsize > 0, "No matches");   
    check_mem(targets);

    for (i = 0; i < bufsize; i++) {

        FILE *contents = fopen(globbuf[i], "r");

            if (contents == NULL) {
            continue;  
        }   

        if (logic == OR) {
            rc = or_match(targets, contents);
            if (rc == 0) {
                printf("%s\n", globbuf[i]); 
            }       
        } else {
            rc = and_match(targets, contents);
            if (rc == 0) {
                printf("%s\n", globbuf[i]); 
            }
        }

    }

    return 0; 
error:
    return 1;
}


int main (int argc, char *argv[])
{
    int rc = 0; 
    int i = 0;
    char *targets[argc];
    char *fpatterns[100];
    glob_t globbuf;

    char *logdir = concat(getenv("HOME"), "/../../Library/Logs");   
    check_mem(logdir); 

    check(argc > 1, USAGE);

    rc = parse_args(targets, argv, argc);   
    check(rc == 0 || rc == 2, "Argument Parsing Error");
    if (rc == 2) {
        goto cleanup; 
    }   

    rc = ldpatterns(fpatterns);
    check(rc == 0, "Pattern Loading Error");

    rc = chdir(logdir);
    check(rc == 0, "Directory Error");

    for (i = 0; i < patterns; i++) {
        rc = glob(fpatterns[i], GLOB_NOCHECK | GLOB_APPEND, NULL, &globbuf);
        check(rc == 0, "Glob Error");   
    }

    rc = process_results(targets, globbuf.gl_pathv, globbuf.gl_pathc);


    /* Freeing Allocated Memory */  

cleanup:    
    if (logdir) {
        free(logdir);
    }

    if (globbuf.gl_pathc > 0) {
        globfree(&globbuf); 
    }

    for (i = 0; i < argc; i++) {
        if (targets[i]) {
            free(targets[i]); 
        }
    }

    for (i = 0; i < patterns; i++) {
        if (fpatterns[i]) {
            free(fpatterns[i]); 
        }
    }

    return 0; 

error:
    if (logdir) {
        free(logdir); 
    }

    if (globbuf.gl_pathc > 0) {
        globfree(&globbuf); 
    }

    for (i = 0; i < argc; i++) {
        if (targets[i]) {
            free(targets[i]); 
        }
    }

    for (i = 0; i < patterns; i++) {
        if (fpatterns[i]) {
            free(fpatterns[i]); 
        }
    }

    return 1; 
}

test.sh

    #!/bin/bash

    test_amount=0
    test_passed=0

    header()
    {
        echo "\x1b[36m ********** $1 ********** \x1b[0m\n"
    }

    info()
    {
        echo "\x1b[33m ---------- $1 ---------- \x1b[0m\n"
    }

    run_test()
    {
        test_amount=$((test_amount+1))

        echo "\x1b[30mOUTPUT: \x1b[0m"
        $2 
        echo "\n\x1b[30mRESULT: \x1b[0m"

        $($2 > out.txt 2>&1) 

        if grep -e ERROR out.txt ; then
            if [[ -z "$3" ]]; then
                echo "$1 => \x1b[31mFAILED\x1b[0m\n"
            else
                echo "$1 => \x1b[32mPASSED\x1b[0m\n"
                test_passed=$((test_passed+1))
            fi  
        else 
            if [[ -z "$3" ]]; then
                echo "$1 => \x1b[32mPASSED\x1b[0m\n"
                test_passed=$((test_passed+1))
            else    
                echo "$1 => \x1b[31mFAILED\x1b[0m\n"
            fi
        fi  
        $(rm out.txt)

        info "***"  
    }

    show_results()
    {
        echo "\x1b[30mTOTAL TESTS: $test_amount\x1b[0m\n"
        if [[ $test_amount -eq $test_passed ]] ; then
            echo "\x1b[32mALL TESTS PASSED.\x1b[0m"
        else
            echo "\x1b[30mTESTS PASSED:\x1b[0m \x1b[31m$test_passed\x1b[0m\n"
        fi
    }

    make build

    header 'TEST START' 

    run_test 'No Args' './build/logfind' err 

    run_test 'Bad Arg No Filename' './build/logfind -r' err

    run_test 'Bad Arg With Filename' './build/logfind -r something.txt' err 

    run_test 'Good Arg With Filename OR' './build/logfind -o something.txt'  

    run_test 'Good Arg With Filename AND' './build/logfind something.txt' 

    run_test 'Matching File' './build/logfind -o parallels' 

    make clean 
    show_results
    header 'TEST FINISH'

dbg.h

    #ifndef __dbg_h__
    #define __dbg_h__

    #include <stdio.h>
    #include <errno.h>
    #include <string.h>

    #define ANSI_COLOR_RED     "\x1b[31m"
    #define ANSI_COLOR_GREEN   "\x1b[32m"
    #define ANSI_COLOR_YELLOW  "\x1b[33m"
    #define ANSI_COLOR_BLUE    "\x1b[34m"
    #define ANSI_COLOR_MAGENTA "\x1b[35m"
    #define ANSI_COLOR_CYAN    "\x1b[36m"
    #define ANSI_COLOR_GREY    "\x1b[30m"
    #define ANSI_COLOR_LPURPLE "\x1b[35m"
    #define ANSI_COLOR_RESET   "\x1b[0m"

    #ifdef NDEBUG
    #define debug(M, ...) 
    #else
    #define debug(M, ...)  fprintf(stderr, "%s[DEBUG]%s %s%s%s:%s%s:%d%s: " M 
    "\n", ANSI_COLOR_GREEN, ANSI_COLOR_RESET, ANSI_COLOR_GREY, __FILE__, 
    ANSI_COLOR_RESET, ANSI_COLOR_LPURPLE, __func__, __LINE__, ANSI_COLOR_RESET, 
    ##__VA_ARGS__)
    #endif

    #define log_test(M) fprintf(stdout, "%s[TEST]%s %s => " M "\n", 
    ANSI_COLOR_CYAN, ANSI_COLOR_RESET, __func__);  

    #define clean_errno()  (errno == 0? "None" : strerror(errno))

    #define log_err(M, ...)  fprintf(stderr, "%s[ERROR]%s (%s%s%s: %s%s:%d%s: 
    %serrno%s: %s%s%s) " M "\n", ANSI_COLOR_RED, ANSI_COLOR_RESET, 
    ANSI_COLOR_GREY, __FILE__, ANSI_COLOR_RESET, ANSI_COLOR_LPURPLE, __func__, 
    __LINE__, ANSI_COLOR_RESET, ANSI_COLOR_RED, ANSI_COLOR_RESET, 
    ANSI_COLOR_GREY, clean_errno(), ANSI_COLOR_RESET, ##__VA_ARGS__)

    #define log_warn(M, ...)  fprintf(stderr, "%s[WARN]%s (%s%s%s: %s%s:%d%s: 
    %serrno%s: %s%s%s) " M "\n", ANSI_COLOR_YELLOW, ANSI_COLOR_RESET, 
    ANSI_COLOR_GREY,  __FILE__, ANSI_COLOR_RESET,  ANSI_COLOR_LPURPLE, 
    __func__, __LINE__, ANSI_COLOR_RESET, ANSI_COLOR_RED, ANSI_COLOR_RESET, 
    ANSI_COLOR_GREY, clean_errno(), ANSI_COLOR_RESET, ##__VA_ARGS__)

    #define log_info(M, ...)  fprintf(stderr, "%s[INFO]%s (%s%s%s: %s%s:%d%s) " 
    M "\n", ANSI_COLOR_BLUE, ANSI_COLOR_RESET, ANSI_COLOR_GREY, __FILE__, 
    ANSI_COLOR_RESET, ANSI_COLOR_LPURPLE,  __func__, __LINE__, 
    ANSI_COLOR_RESET, ##__VA_ARGS__)


    #define check(A, M, ...)  if ( !(A) ) { log_err(M, ##__VA_ARGS__); errno=0; 
    goto error; }

    #define sentinel(M, ...)  { log_err(M, ##__VA_ARGS__); errno=0; goto error; 
    }

    #define check_mem(A)  check((A), "Out of memory.")

    #define check_debug(A, M, ...)  if ( !(A) ) { debug(M, ##__VA_ARGS__); 
    errno=0; goto error; }


    #endif
\$\endgroup\$
  • 1
    \$\begingroup\$ Your dbg.h seems to have aquired spurious newlines breaking it badly... \$\endgroup\$ – Deduplicator May 11 '17 at 19:15
  • \$\begingroup\$ My apologies. This is a modified version of the dbg.h presented in the book. It adds color to the output. I will fix the formatting and edit my post. Thank you for your comment! \$\endgroup\$ – user138602 May 11 '17 at 23:23
3
\$\begingroup\$
  • static char logic = AND;

    AND is enum Logic, so logic (logically) shall be declared as

        `static enum Logic logic = AND;`
    
  • (char *) malloc

    serves no purpose, and may hide a serious problem. Never cast the result of malloc.

  • Trust the standard library

    • The malloc/strncpy combo is a long way to say strdup.
    • There is no reason to call strncpy (which is almost always wrong thing to do); since you already called strlen, you must assume that the string is well-formed, so just strcpy would suffice.
  • goto

    on error is only justified if the function need to perform multiple cleanups. Functions that do not perform any shall return immediately.

  • Avoid globals

    Globals make a program really hard to read and maintain. patterns is a natural return value of ldpatterns.

    While we are on ldpatterns, you should fclose(dirfile) at the end.

  • strfmatch

    wastes time matching through the entire file. It should return success as soon as the first match found.

  • __int vs size_t

    The cast like match == (int)tcont_size usually indicates the problem. I suspect it was added to silence the warnings. The real fix would be to declare size_t match;.

  • Returning condition

    The

    if (match == (int)tcont_size) {
        return 0;
    } else {
        return 1;
    }
    

    is a long way to say

    return match != tcont_size;
    

PS: There might be more problems which escape an immediate review. All that said I have huge reservations about Learning C the Hard Way. The book IMHO is seriously misguided.

\$\endgroup\$
  • 1
    \$\begingroup\$ Thank you very much for taking the time to review my code. This is my first time messing with C and I have to get better at catching those silly mistakes that you mentioned. I'm still getting used to the C way so to speak. However, I found conflicting views on the use of strdup as it is actually not part of the standard library. Additionally, I would like to know why you think LCTHW is misguided? Thank you in advance for your input. \$\endgroup\$ – user138602 May 11 '17 at 22:39
  • \$\begingroup\$ @raysd Addressing problems with LCTHW will take another book, or at least a series of blog posts. The major point is that the tasks and examples on which the book is built are not suited for C. It is much easier to solve these problems in Java, Python, Perl, you name it. OTOH the problems where C really shines are not addressed. Besides, there are plain mistakes. As a C beginner, read K&R. Seriously. \$\endgroup\$ – vnp May 12 '17 at 1:47
  • \$\begingroup\$ I understand. Thank you very much for your input. I'm planning on picking up K&RC once I've messed around with C for a while. \$\endgroup\$ – user138602 May 12 '17 at 1:51

Your Answer

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