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 ...
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) ...
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 ...
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("%...
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 ...
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 ...
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. ...
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 ...
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:
...
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 ...
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 ...
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;
...
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 ...
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 ...
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 ...
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 ...
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
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 ...
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 ...
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
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
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,
...
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 ...
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() ...
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
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 ...
Only top voted, non community-wiki answers of a minimum length are eligible