New answers tagged

0

No test reference There are no tests in test_str_join.c, just code being exercised. How is the test to detect if output is correct? If by visual inspection, including comments of expected output would at least be a good start. Undefined behavior printing a null pointer Example: last test may print a = (null), strlen(a) = 18446744073709551615 or crash or ... ...


0

In addition to @chux's comments, I'd point out that you don't need a pass through the hash-table. You can simply keep track of the current highest count, and the corresponding word. By doing this you'd also avoid any need for MAX_WORD_LEN...


1

Good formatting Inconsistent Out-of-memory handling After words_to_counts = ht_create();, code tests and on failure, prints an error message. Yet not so with char* dup_sentence = strdup(sentence); I recommend in all such places that allocation may fail, simple return NULL. Let the the caller print an error if desired. Artificial limitation Yes, ...


2

Code looks pretty good. I prefer some other style guide, but this looks consistent, and consistency is important too. Indentation Arbitrarily, you can spell the string constant like this: char* input = "mat1(3, 3) = " "( 1.0, -3.0, 5.0, " "2.0, -1.0, 5.0, " "1.0, -3.0, 5.0 )"; Placing the similar tokens ...


1

Given that all the values are between 7 and 13, we can implement is_prime() much more efficiently: bool is_prime(int n) { switch(n) { case 7: case 11: case 13: return true; default: return false; } } In removePrimes(), we allocate and reallocate memory without checking for failure. Always test the return value from allocation ...


4

Using C99 Variable Length Arrays to save calculated string-lengths, and fixing all the other problems chux identified, your code can be made far more efficient: char* astrcat(const char* restrict delim, size_t num_args, ...) { if (!num_args) return calloc(1, 1); size_t lengths[num_args]; const size_t ndelim = delim ? strlen(delim) : 0; ...


8

Reserved identifiers The include-guard _STRCAT_NEW_H_ is one of the names reserved for the implementation, as it begins with underscore and an upper-case letter. As a general guide, never begin names with underscore. The function name beginning with str is reserved for standard library extension, so choose a different name there, too. Interface Consider ...


17

Neat and well formatted Design Rather than "then after every string, the 'delim' string is concatenated." (that sounds like n delimiters), I would expect between every string. between, or the like, matches code (n - 1 delimiters). Design: num_args == 0 Consider allowing this corner case. // if (num_args <= 0) return NULL; if (num_args < 0) ...


6

I didn't read the whole code thing, just looked at some parts. Also, Andreas already mentioned some of the major things, like making one system call per byte being a performance disaster. Some of those local arrays, especially char fullResp[100000];, are a bit large to keep on the stack. Most mainstream systems give you at least 1MiB of stack space (e.g. ...


18

Calling strcat in a loop is inefficient The function strcat works the following way: It must first search the entire the destination string until it finds the terminating null character at the end. It then appends the source string. Therefore, calling the function strcat in a loop in order to append one character at a time is highly inefficient. This is ...


2

Redundant code getchar_eof() essentially mimics getchar() and feof(). It fails to work well when ferror() is true or stdin is re-opened. getchar() already returns EOF when stdin end-of-file flag is set. Instead use, getchar(), feof(), ferror(). Bug int getchar_eof(void) returns junk (indeterminant data) when a rare input error encountered as the return ...


2

I would like to know if this is the best way to go about this Over all: Good journeyman effort for printing. Unusable for reading. Non-C syntax Code looks wrong as it appears that input() takes 2 arguments: a char * and an (uninitialized) int. Instead, via the underlying macro, it is the & number that is used. I suspect C++ influence here. int ...


3

Functional difference from fgets() I'd expect get_input_from_stdin_and_discard_extra_characters() to perform like fgets() when there is no input. Instead get_input_from_stdin_and_discard_extra_characters(str, ...) returns str. fgets(str, ...) returns NULL. Types Size of buffer and array indexing is more idiomatic C as size_t than long. Even int has some ...


2

I found this very interesting and was not aware of the _Generic operation in C. The following are really more questions than suggestions. Is there any specific reason for including tgmath.h? The do ... while(0) construct seems odd to me. It looked like you used to force the inner part into a scope of its own. Is there any specific reason why you could not ...


2

This looks like a missed opportunity. I was hoping to see something that would usefully handle re-trying when the input can't be parsed, but instead we have a macro that throws away the return value from scanf(), so that we have no idea whether it was successful or not. I'm not a fan of headers that change their behaviour according to what macros are ...


0

I dislike else if where the preceding controlled statement diverted program flow unconditionally (continue/break/return). With a statement like if (c1) modify O // not touching X else if (c2) modify X // not touching O , don't continue if (c3(O)) sO if (c4(X)) sX (calculate_win_col/line()): switch (board[j][k]) { case O_PLAY: ...


3

substr This function does not work as I expected it to. If I called it like in the following: substr("abc", 1, 1); I would have expected it to return an empty string "" and not "b". You should try to limit your function to do a single task. substr allocates a new string and writes to it. You can leave the allocation up to the ...


8

get_input_from_stdin_and_discard_extra_characters c = getchar(); c is a char, so we lose the ability to distinguish EOF here. We need to use int for that. The whole function could be made much simpler: just use fgets(), then if the final character read was \n, overwrite it, else discard the rest of the line with scanf("%*[^\n]"),scanf("%...


1

Know your standard library There's a standard function for finding one of a set of characters, and that's strcspn(). int password_check(const char *const password) { const size_t len = strlen(password); return strcspn(password, "ABCDEFGHIJKLMNOPQRSTUVWXYZ") == len || strcspn(password, "0123456789") == len || !...


1

Use the correct types Use size_t instead of int32_t for the array size and index. Using int32_t will limit it to arrays containing 2,147,483,647 elements even if you compiled it for 64-bit mode. Also, note that it does not make sense to check if i or length is less than zero since it is unsigned. Node* array_to_list(int array[], size_t length) { // ... ...


1

get_error_string() needs to return const char*, since it returns pointers to string literals. It should probably be implemented using switch. My preference is not to have a default case when dealing with enumerations (which the errors should be), so that I can have a compiler warning when I miss one of the values. enum dll_error { NO_ERROR = 0, ...


4

You can run into problems with thread safety when using a static buffer. I prefer the following form which should also work for any size display: void lastDigits(char* buf, const size_t size, const char* num) { size_t len = strlen(num); if (len >= size) { memcpy(buf, num + len - size, size + 1); } else { memset(buf,...


4

Well, there are some concerning points about your code: You are not using const where appropriate. It is there to help you avoid obvious bugs, like overwriting constants. Crank up the warning-level, and the compiler warns about assigning string-constants to mutable pointers. If you need a small persistent space, ask for it. Don't misuse string-constants: ...


1

I don't understand what this is doing. I see it takes each successive element and if it's not already greater than the previous last element, I expect to insert it into the correct position: that is the definition of Insertion Sort. Instead, it appears to move it all the way to the left, to element zero. Then the whole sort is inside another while loop that ...


1

Avoid subtraction with unsigned math When message[0] == 0 and size_t len = strlen(message) - 1;, len takes on the value of SIZE_MAX. Instead: void sub_shift(char *message, char mode) { // size_t len = strlen(message) - 1; size_t len = strlen(message); if (mode == MODE_1) { // for (size_t letter = 0; letter < len; letter++) for (size_t ...


1

/* Same as mandatory, only length comparison check is removed */ Don't do that. You duplicated the entire (complex) function to make minor changes; specifically to skip error checks. Write one copy of the function. Put the optional checks in another preliminary function that's called first, or pass a boolean parameter to enable/disable the tests. You ...


4

We start off reasonably well, defining some constants: #define DECK_SIZE 52 //to be used to randomly select each of 52 cards #define SUITS_PLUS_ONE 5 //to be used to generate any of the 4 suits #define PIPS_PLUS_ONE 14 //to be used to generate any of the 13 pips I'd argue that we shouldn't have _PLUS_ONE constants, but to simply add 1 where ...


0

malloc() can return a null pointer This code hides Undefined Behaviour: char *file_contents = malloc(sizeof(char) * file_length); // dynamically allocated because of file size fgets(file_contents, file_length, fp); // write the stream to the variable We need to ensure that file_contents isn't a null pointer before we pass it into fgets. Also, ...


1

Bug: failure to read entire file fgets(file_contents, file_length, fp); only reads up to a line. Research fread(). Bug: insufficient allocation fileCount does not account for the appended NULL. // retArr = malloc(sizeof(*retArr) * fileCount); retArr = malloc(sizeof *retArr * (fileCount + 1)); ... retArr[logFilePos + 1] = NULL; Bug: 257 vs 256 int fgetc() ...


2

Pick a style You write: int get_file_char_count(FILE *f, char *filename); int searchForKeyword(char *keyword, char *check_string); You are mixing snake_case with camelCase names for your functions. I suggest that you stick with a single naming case for categories of names, so that all functions will use the same style. Pick one and use it everywhere. Parse ...


2

First thoughts: all the code is in main(), but we would like to have a reusable function. I'd create a function that accepts an array and its length: void insertion_sort(int arr[], size_t len) Here is a good demonstration of why one declaration per line is a good style to follow: int numbers[25]={ … }, temp_str, comp_count=0 ; In the full line, temp_str ...


1

Caution: Assessment of advanced matrix multiplication is non-trivial. See Introduction to Performance Engineering & Matrix Multiplication for starters, in particular the part about cache access patterns and similarities therein between tiling and recursive decomposition on one hand and Strassen&Cie․ on the other. Public/main items of a program ...


5

Consider double Rather than int and float objects, consider double. Printing floating point Use "%e" or "%g" to see more information. "%f" prints many large values with dozens of uninformative digits. "%f" prints small values as 0.000000. Using "%f" takes the float out of floating point. // suggestions ...


0

Result Thank you all for your answers. :) I think I've come to a really satisfactory result to me. I've learned a lot. In this stadium I think I'm done with this program unless there's some bug I don't know about (may be, if I know myself). Rewrote the original program to making it a running script instead of reading the result from stdin. Used struct to ...


4

You should know that double is the normal floating-point type, and float is half sized and should only be used where you really need to save the space. Your program is mostly dealing with reading input, which is not the problem you came to solve! A real utility program would take command line arguments, not prompt the user to type things. That is, you ...


1

You are definitely overusing, and misusing assert(). The problem is that it is something that should become a no-op when your code is compiled for release. As such, it should have NO side effects (like calling bind() or listen(), and should not be used for testing environmental errors. assert() should only be used for testing for programming errors. This ...


1

I hope I am not overusing assert calls You do. assert is a debugging instrument. If NDEBUG macro is defined, the calls to assert are removed at the compile time. This is a usual practice for the production code. #define whaterror strerror(errno) I advise against it. The macro adds no value, and only obfuscates the code. memset(malloc Keep in mind that ...


6

set_number_from_stdin has several bugs: if fgets fails, you assign the uninitialised variable final_number to *num. That’s undefined behaviour. In practice a compiler might assume that UB never happens, and it might thus for instance remove the break statement entirely. You need to handle the input error. Since your current function has no way of signalling ...


10

Overall clear and readable. Some remarks: atoi is one of those standard library functions that should never be used. It is 100% equivalent to strtol family of functions, except that the latter have error handling and support for other number bases than decimal. Since you are using this function on user input which has not been sanitized, you should not use ...


2

Big task OP's goal of "trying to write a generic dynamic array which is type safe." is admirable, but not a good task for someone new to C. I recommend to start with a write a generic dynamic array for void *. Later, research _Generic. Not so generic Approach relies on types not having spaces. Try struct DynArray(long long) llArr; Stand-alone ...


Top 50 recent answers are included