Hot answers tagged

9

Headers I don't see any need for <string.h>. We can drop <stdbool.h> if we rewrite check_not_equal: #define check_not_equal(a, b) ((a) != (b)) Personally, I wouldn't use a macro for that - it just obfuscates the code. <stdio.h> is used only by the tests. We may compile them separately if we want to use the function in other programs. And ...


8

You do leak memory because you forgot to free hash_table in destroy_hash_table(). To correct the issue, you need to free the allocated hash table after freeing all allocated nodes, e.g. void destroy_hash_table(node **hash_table, int n){ for (int i = 0; i < n; i++) { node *head=hash_table[i]; node *tmp=head; while (head) ...


7

Your C code is not too bad, however you seem not to know much about hash-tables, and since you requested a performance review, I will focus on a brief (and incomplete) exploration of the various ways of designing hash-tables and their performance implications. Performance The performance of hash-tables is a widely discussed topic; various benchmarks will ...


5

This answer only addresses the original spinlock version in the question. Ensure the code is self-explanatory Before reading the code, I recommend running it first and see how each state is encoded. Rather than me trying to explain the pattern, I think you'll be quicker to catch the pattern by actually looking at the output of each state before a task has ...


4

Make the tests self-checking It's good that we have a test program that allows us to exercise the code (although I would drop the non-portable and unnecessary system("clear")). I think it's better if we have an automated test that doesn't require user input. We could do this by having a script drive the existing test program, but I think it's ...


4

Nice to see the use of the standard EXIT_SUCCESS and EXIT_FAILURE. The return value from the rename() system call can actually be a check against EXIT_SUCCESS because that system call is returning either EXIT_SUCCESS or EXIT_FAILURE. From main() Use Return Rather Than Exit Since all of this is happening in the main() function there is no reason to use the ...


3

Allocate to the size of the referenced object, not type Cast not needed in C. sizeof *the_pointer is always the correct type. sizeof(int) relies on matching the type - which may change with updates and is harder to review when the type is far from the malloc() line of code. Lead with sizeof ... to insure math used is at least size_t. Makes no difference ...


3

Overall Aside from the atrocious format, very nice code for a learner. Use an auto-formatter! Avoid manual formatting, yet always run an auto-formatter before code review. Ding, Ding, Ding!!! No warnings, even with many enabled. Yeah! Bug: direction never assigned before use. char direction; ... control(direction,&snake[0][0],&snake[0][1]); Avoid ...


3

Any better ideas? Consume the buffer Rather than leave the buffer "as-is" while parsing, adjust its start and end while parsing. char *s = httpHeader; s = parse(s, "Content-Length", "\n"); if (s == NULL) Handle_Error(); s = parse(s, ":", "\r"); if (s == NULL) Handle_Error(); long contentLenVal = strtol(s, .....


3

Remove unnecessary comments You have added a lot of comments to your code, most of them are however unnecessary. For example, why is there a huge banner saying "START OF FILE" at the start of the file? It states the obvious, but takes up valuable space. Just remove it. Explaining the C language in comments is not helpful. Maybe if you are an ...


2

Your code is OK, for a beginner, so please don't worry too much about these comments. I bet you program is fine until you enter the data "break me". The reason I can list these points so easily, is because I've done them more times than I care to think of, and I've probably done some of them today. Code Review is always a case of do what I say, ...


2

Names are long and contain spaces The below is poor as it 1) does not limit input, allowing a buffer overflow. never user "%s" from scanf() without a width limit like "%24s" 2) does not check the return value of scanf() - how did you know user input was successful? 3) does not handle names like "Betty Jo" 4) Assumes the surname ...


2

Never cast what malloc returns. It is not necessary, and may lead to hard-to-find bugs. strcpy(), strcat(), strcat() sequence is anti-idiomatic. Prefer sprintf(new_source, "%s/%s", current_directory, source); Prefixing the source and destination with PWD seriously limits usability of the program: it can't deal with absolute paths at all. Along ...


2

Your code is too difficult to read. Please do some good indentation and formatting. You can't solve this without storing whole information somewhere, because you can fight dragon in any order, so in each step you have to choose remaining dragon that has weakest hurdle but gives largest strength bonus.


2

Syntax words used in functions I myself don't like the use of a syntax word in a function name like return_base64(). This is due to the fact that some older code highlighting editors may change color of the part return. Despite the fact that it is in a function definition, many older editors are unable to distinguish it. When declaring function names avoid ...


2

You can separate input and processing. And worst case, looking at the input values before the last one is read doesn't help you any: The last dragon may be the only one Kirito initially dominates. Then again, one tactic is to reap the low-hanging fruit: If the dragon is inferior, just accumulate strength. If the dragon is too strong now, queue it up. ...


2

if (*ptr == '\n') is very opportunistic, and leads to a serious problem. An input like \n results in a segfault. You should honestly tokenize any input, and only then test what args[0] is. It is easy to overflow an args array (try a a a ... for more than 64 times). A tokenizing loop should hard stop at 63rd token. ptr is not really used anywhere. ...


1

Error messages should go to the standard error stream: fprintf(stderr, "Usage: %s message\n", *argv); return 1; No need to flush any standard stream here, as returning from main() does that as part of exiting the program. In the child side, we throw away the return value of read() after we compare with -1. However, we need to use ...


1

Variable Names In 6 months time the one and two letter variable names won't mean anything to you or anyone else that needs to maintain the code. Use descriptive variable names and make the code as self documenting as possible. Declare the Variables as Needed In the original version of C back in the 1970s and 1980s variables had to be declared at the top of ...


1

There are many things besides properly formatting the code that would improve it. Complexity The function main() is too complex (does too much). This isn't clear in this program because it is simple, but as programs grow in size the use of main() should be limited to calling functions that parse the command line, calling functions that set up for processing, ...


1

My initial impression when I was only visually scanning the code was Well Done! The code is very readable, the variable names and function names are clear and state the purpose of the variable or function clearly. This code should not work. Warnings from my compiler: main.c(147,20): warning C4456: declaration of 'i' hides previous local declaration main.c(...


1

Reducing maintenance burden: void drawBoard(gc*, int); void updatePieces(gc*, int[8][8], int, const char*[12]); int findMousePosX(int[8][8], int, int, int, SDL_Event); int findMousePosY(int[8][8], int, int, int, SDL_Event); We could avoid having to maintain these declarations by moving the function definitions here instead. Declare variables as locally ...


1

The major flaw in the approach is that there's no guaranteed delivery of signals. If the server doesn't consume a signal quickly enough, it can miss a subsequent delivery of the same signal. Since the protocol provides no means of error detection/correction or resynchronisation, that will result in corrupt data. This situation is most likely to occur on ...


1

Use an auto-formatter Bugs: string Never use scanf("%s",...); without a width limit like scanf("%5s",password);. The width must be less than the array count. char password[6]; ... scanf("%s",password); ... if (strcmp(password,"252558")==0) For strcmp(password,"252558")==0 to be true, more than 5 ...


1

Use floating point Rather than convert to int and risk problems when the float value is outside the int range, use fmodf() via <math.h>. case '%' : rslt = fmodf(num1, num2); printf("\n%f % %f = %.3f\n\n", num1, num2, rslt); break;


1

regarding: void main() results in: untitled.c:3:6: warning: return type of ‘main’ is not ‘int’ [-Wmain] There are also 3 other compiler warnings: gcc -ggdb3 -Wall -Wextra -Wconversion -pedantic -std=gnu11 -c "untitled.c" -o "untitled.o" results in: untitled.c:3:6: warning: return type of ‘main’ is not ‘int’ [-Wmain] 3 | void main() | ...


1

double *x; double *y; int i; x =(float*)malloc(11*sizeof(float)); y =(float*)malloc(11*sizeof(float)); Your x and y are pointers to doubles, but you are allocating space for 11 floats. Since floats are smaller than double you are writing over the boundaries of the memory allocated in x and the allocated memory of y happens to be just next to it, so you end ...


1

This [looks] a lot of code but pop()/eject() and push()/inject() are very similar - due to code duplication! don't write, never publish uncommented/undocumented code don't leave "allocation values" unchecked OK, so C's exception handling is nothing to speak of, and trying to illuminate not repeating oneself, I fail to usefully handle allocation ...


1

Use the Right Kind of Inter-Process Communication If you need to store data persistently and load it into memory, you can save it to a memory-mapped file, map it into another process’ memory with mmap(), and read and update the value. There is no need to keep a daemon alive just to remember the value. If you need two processes to share memory while both are ...


1

Why is there a definition of CXX when there's no C++ sources? That very long WARNINGS = line is hard to read. Split it over several lines, using +=. Remove the flags that are implied by -Wall (namely, -Wchar-subscripts -Wimplicit, -Wmain, -Wmissing-braces, -Wparentheses, -Wsequence-point) or by -Wpedantic (-Wmain, -Wpointer-arith). I don't see the need ...


Only top voted, non community-wiki answers of a minimum length are eligible