HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Learning C The Hard Way - logfind implementation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thehardwaylearningimplementationlogfind

Problem

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
#include
#include
#include
#include
#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

Solution

-
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.

Code Snippets

`static enum Logic logic = AND;`
if (match == (int)tcont_size) {
    return 0;
} else {
    return 1;
}
return match != tcont_size;

Context

StackExchange Code Review Q#163097, answer score: 3

Revisions (0)

No revisions yet.