[Here][1] I asked this question: > My goal is to go through the [list of all English words][2] (separated by > `'\n'` characters) and find the longest word which doesn't have any of > these characters: "gkmqvwxz". And I want to optimize it as much as > possible I updated the code with the help of suggestions from answers, but I still need comments on this updated version Changes: 1. Name of the file and the forbidden characters are no longer hard-coded. They are passed by arguments. 2. Added several error checks. 3. Used pointers instead of indexes. 4. `buffer` is freed when we're done with it. 5. Used `bool` instead of `int` for the return type of `is_legal`. 6. Parameters to `is_legal` are made `const` since we don't change them. 7. Skip next lines (`'\n'`) remaining from previous lines. 8. Added some functions to keep main simple. 9. Removed superfluous headers (`#include <string.h>`, `#include <stddef.h>`, `#include <unistd.h>`). 10. `is_legal` need not know about the entire `buffer`. Just the relevant pointers are now sent. 11. `length` is no longer fixed. We get the size of the array at runtime 12. `buffer` is terminated with null. Updated code: #include <ctype.h> #include <stdlib.h> #include <stdio.h> #include <stdbool.h> static inline bool is_legal(const char* beg, size_t size, const char* bad) { for (; size-- !=0 ; ++beg) { /* go through current word */ char ch = tolower(*beg); /* The char might be upper case */ for (const char* bad_ptr = bad; *bad_ptr; ++bad_ptr) if (ch == *bad_ptr) /* If it is found, return false */ return false; } return true; /* else return true */ } static inline size_t get_next_word_size(const char* beg) { size_t size = 0; /* resulting size */ for (; beg[size] && beg[size] != '\n'; ++size) /* read the next word */ { } /* for loop doesn't have a body */ return size; } static inline char* get_buffer(const char* filename) { char *buffer = NULL; /* contents of the text file */ size_t length; /* maximum size */ FILE* fp; fp = fopen(filename, "rb"); if (!fp) { /* checking if file is properly opened */ perror("Couldn't open the file\n"); return NULL; } if (fseek(fp, 0, SEEK_END)) { perror("Failed reading"); return NULL; } length = ftell(fp); if (fseek(fp, 0, SEEK_SET)) { perror("Failed reading"); return NULL; } buffer = malloc(length + 1); /* +1 for null terminator */ if (buffer == NULL) { /* checking if memory is allocated properly */ perror("Failed to allocate memory\n"); free(buffer); return NULL; } fread(buffer, 1, length, fp); /* read it all */ fclose(fp); buffer[length] = '\0'; /* terminate the string with null*/ return buffer; } int main(int argc, char **argv) { if (argc < 3) { printf("Usage: FileName BadChars"); return 0; } char* filename = argv[1]; char* badchars = argv[2]; char *buffer = get_buffer(filename); if (buffer == NULL) { return -1; } const char *beg = buffer; /* current word boundaries */ size_t size = 0; const char *mbeg = beg; /* result word */ size_t msize = 0; while (beg[size]) { beg += size + 1; /* +1 to skip the '\n' */ size = get_next_word_size(beg); /* get the size of the next word */ if (size > msize && is_legal(beg, size, badchars)) { /* if it is a fit, save it */ mbeg = beg; msize = size; } } printf("%.*s\n", msize, mbeg); /* print the output */ free(buffer); return 0; } I would especially appreciate comments regarding the way the code reads the entire file into a single dynamically allocated array. About if and how it could be improved. I wouldn't like to sacrifice the performance, but some "best practices" especially about this part are very welcome. [1]: https://codereview.stackexchange.com/q/211366/183642 [2]: https://github.com/dwyl/english-words