Hot answers tagged

13

There is a rather large amount of code here and a lot of macro trickery, so I shall refrain from attempting to comment on it all. There are, however, a few things that immediately jumped out at me. Don't use new as your allocating macro. That's extremely confusing, and it's going to make compiling it as C++ rather difficult. (Not that having C be ...


8

Very impressive! A few notes: I really like how you are using C11 stuff. #define safe_add(x, y) _Generic((x + y), \ unsigned int: safe_add_uint, \ unsigned long: safe_add_ulong \ )(x, y, __FILE__, __LINE__) It's good that you are using the more modern ...


7

Some small stuff: sgl_detail_current_exception is initialize to 0 as it is in global memory. Given the enumerated list starts with -1, then sgl_logic_error == 0: sgl_exception_t sgl_detail_current_exception; // is effectively sgl_exception_t sgl_detail_current_exception = sgl_logic_error; Recommend adding an enumerated value of sgl_none = 0 to ...


7

Variable initialisation in condition Both here: uint8_t rv; if ((rv = initialize_glfw())) DIE("%s%" PRIu8 "\n", "initialize_glfw() failed with code ", rv); here: GLFWwindow *window; if (!(window = glfwCreateWindow( image.width < screen_width ? image.width : screen_width, image.height < screen_height ? image....


7

I did manage to improve some things since the question was posted. So here is what I discovered, that could somehow improve the implementation and the usability of the Vector(T): First of all, there was an error in the code. Shame on me. I shared a piece of code that could not be compiled. I must have posted a version that was not up-to-date. The macro new ...


6

You will have to write many functions that look the same, but unless you use more macros to generate the code, this is how C works when you want genericity. You should consider factoring out the parts of the functions that are likely to be repeated, such as your error message: unsigned int safe_add_uint(unsigned int x, unsigned int y, const ...


5

The good The code uses library functions such as argp_parse() so it's obvious you're not trying to reinvent the wheel. Some of the functions already follow SRP as noted below. Reduce function complexity The Single Responsibility Principle (SRP) is a good thing to keep in mind. This website provides a good list of principles to keep in mind when designing ...


5

The first thing is that most of the relevant code is squeezed into a single large method parse_cmd_args. That is code granularity is definitely insufficient. The reasonable solution would be to extract methods that form fancy inner loops. "printf debugging" as error handling mechanism is no good at all. It may provide some kind of human-readable description ...


5

There are two main things to talk about: checking the input, and buffer handling (your malloc question). It's a very bad idea to do things like array[i + 1], unless you're absolutely, 100% sure that this refers to memory that is both allocated and part of the string. If it's not allocated, your program is begging for a crash; if it's not part of the string, ...


4

Welcome back to C Binary constants are not part of standard C - yet // 0b10000000 0x80 Lack of error detection i += 2;, i += 3 or i += 4 in utf8_strlen() assumes the characters skipped over are of the proper form 0b10...... Instead test for that. utf8_strlen() then needs some way to convey an error. Perhaps utf8_string_size(), which includes the space ...


4

Portability In calculations like c[n] = (text[i] & 0b00000111) << 18 | (text[i + 1] & 0b00111111) << 12 | (text[i + 2] & 0b00111111) << 6 | (text[i + 3] & 0b00111111); all operands are promoted to int. That is a 32-bit integer on most platforms, but the C standard only requires that int has at least 16 bits, which means ...


4

Magic numbers The implementation uses them a lot. While the bit-notation helps with indicating what is happening, it doesn't show the intention. What reads clearer: if((text[i] & 0b1000000) == 0) or if((text[i] & UTF8_ONE_BYTE_MASK) == UTF8_ONE_BYTE_COUNT) Error handling It isn't guaranteed that only valid UTF8 strings will be given to this ...


4

Data storage If you change your data_t typedef to be anything other than void*, your linked list implementation has to be copied and modified to handle multiple types (so it's less generic); on the other hand, you get better type safety. The tradeoff is probably in how you're going to use it. One thought: I would suggest looking at how the linux kernel ...


4

Purpose of sgl_detail_in_catch_bloc[]? It looks like the only use of sgl_detail_in_catch_bloc[] is in sgl_throw(): if (sgl_detail_in_catch_bloc[sgl_detail_exceptions_index]) { --sgl_detail_exceptions_index; } But that if statement should never be true because the only place that sets an array entry to true immediately decrements the index as well: ...


4

Warning messages When we print fixed strings, it's better to use plain fputs() rather than the much more complex fprintf(). However, in this case, the diagnostic output should be removed: such side-effects are not part of the contract of memset_s(), and are actively harmful (because the whole point of the checks is to report errors to the calling program, ...


4

Here are some things that may help you improve your program. Prefer const to #define For constants such as TOTAL_BITS, it's generally better to use a named const value rather than a #define. The primary reason is that the const definition enforces type checking, while the macro does not. However, see the next suggestion. Use sizeof for portability The ...


3

Order of pops is strange If I am reading your code correctly, your pop ordering will be different from the push ordering. Suppose we push 1 2 3 4 5 in that order. Your inbox will look like: 5 -> 4 -> 3 -> 2 -> 1 So far so good. But then when we pop for the first time, this is what happens: ret = 5 outbox = 1 -> 2 -> 3 -> 4 Then the ...


3

errno_t memset_s(void *s,rsize_t smax, int c, rsize_t n) You've got a missing space after that first comma. (Sure, inconsistent whitespace doesn't affect functionality; but like any spelling mistake, it indicates that you haven't proofread your code. Which means it probably has some other one-character typos too. Some of which might affect functionality. ...


2

Better Naming Conventions One of the things that immediately jumps out at me is the name you used for the data structure. As you mentioned, a dynamic array is called a List in C# and in Java. However, in the C/C++ world, we generally like to refer to dynamic arrays as either (ahem) dynamic arrays or vectors. When I saw List, my brain automatically started ...


2

Improvements Initializing functions (string_create*) I think it's a common practice that initializing functions return whole structure element. For example, your implementation extern bool string_create_n(string* str, size_t size); should look like this extern string * string_create_n(size_t size); It's also applicable to free/destroy function - this ...


2

Parsing Incoming Messages After calling BIO_read on a 512 byte buffer, you simply set buf[bytes_read] = '\0' (which overflows if you just happen to have read exactly 512 bytes) and start parsing the buffer. However, you have no guarantee that your buffer contains one and only one IRC message. It may contain only the start of a message, multiple full ...


2

Architecture. I'd put the simulation in a stand-alone function leaving the user input in main(). This facilitates code re-use and clearly identifies the data needed to perform and results from the simulation. As @Lundin commented, drop the trailing space from scanf("%d ", &nDays); to scanf("%d", &nDays);. The trailing space obilges the user to ...


2

You ought to check scanf return value and also that the input value is not negative. ++i please.


2

An interesting curiosity, as you say. Because 10 is an exact multiple of 2, all binary fractions have an exact, non-repeating decimal representation (but not vice versa). That means we're guaranteed to terminate. The code compiles cleanly with an aggressive set of warnings enabled, and Valgrind is completely happy with the test program. But I expect you ...


2

You're right to be concerned about allocating memory in the function. One obvious problem is that malloc() can return a null pointer, so we need to check for that before we think about using it: uint32_t* c = malloc(sizeof *c * num_chars); if (!c) { return c; } Making two passes over the input can be problematic, as then we have two pieces of code which ...


1

This module is intended to be written once, never changed again, right? Also, you strive for efficiency? Then put everything in the header. As currently used extern does nothing. Replace it with inline and put the definitions there. You can still move the declarations tothe top of the file, for client convenience. Secondly, one more vote for small object ...


1

Is this the correct way of doing this kind of stuff? Any improvements I can make? The role of member allocation_size is unclear. It appears to represent the raw alignment with h->allocation_size = s;. Later use with stack_head = (char*)stack_head + h->total_size; could then result in an unaligned stack_head. I recommend that void* memPush (size_t ...


1

Generally I would expect a command line parser to set variables that are used by the program to configure the way it works. If the command line parser is used by programs such as ls, cp and other programs that are primarily run from the command line I would expect them to report errors to stderr. Graphic tools might report errors to the console, they would ...


1

On design The original intent was unclear: A memory resource does not worry itself about data that might exist in its buffer; it is up to the user to ensure proper data copying behaviour. Due to the previous point, a memory resource will cause memory leaks on resize and realignment. Since it can't know whether data is in its buffer, it cannot free the data....


1

Bug The padding between block and aligned_ptr depends on how well block was aligned. There is no guarantee that the padding after original malloc is the same as the padding after realloc, so memory_resource_resize may copy the client data to a misaligned location, and therefore the aligned_ptr will not point the start of client data. Realignment Consider ...


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