Skip to main content
Code Review

Return to Answer

some more
Source Link
Art
  • 186
  • 3

Standards

Note also that this project is developed to be compliant with the C11 standard.

Cool, let's find non-standard behavior.

The first thing that jumps out to me is your usage of double underscores.

Both C99 and C11 say (in section 7.1.3):

All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use.

You include <malloc/malloc.h> everywhere. The only operating system where I could find that header file is MacOS. This is definitely not standard.

You seem to be very dependent on the function malloc_size in many places. As far as I know, only MacOS implements it.

You use the function valloc. This is a non-standard function that has been marked as obsolete in 4.3 BSD in 1986. You can get this functionality from a standard (not C, but at least POSIX) by using the posix_memalign function.

Problematic behavior

In cstring_cmp and cstringbuffer_cmp you compare with the shortest string. Why with strncmp which means that you're only comparing the prefixes of the strings (I think, I haven't dug into the exact details of the library) and why not just use memcmp? Especially since what you're not comparing aren't normalC strings because they are not 0円-terminatedanyway.

It's possible to write 0円 into a cstringbuffer with cstringbuffer_write_char (and other functions) that will update the length to include that and later append further characters at the end of the buffers, but you're then using normal strcmp, strncpy, etc. to compare and copy the strings which will not do what you want. Either disallow 0円 explicitly or use functions that don't get confused by it.

allocate_mem has a good prototype with count and size. This is just like calloc and it's what a modern memory allocator should do. But, you miss the point of why calloc does it. The main reason is that you can detect overflows when multiplying count * size. You don't check for overflow in that multiplication which can lead to security issues.

Matters of taste

You're always allocating a struct on the stack, then malloc the same struct, then return a copy. Why? It seems like extra work.

Macros with magic goto error; with the error: label being outside of the macro are horrifying.

You do do { ... } while (0) in macros, this is good style. But you then defeat half the purpose of building macros this way by adding the ; inside the macro. for example if (foo) log_error("foo"); else do_something(); will not compile.

Standards

Note also that this project is developed to be compliant with the C11 standard.

Cool, let's find non-standard behavior.

The first thing that jumps out to me is your usage of double underscores.

Both C99 and C11 say (in section 7.1.3):

All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use.

You include <malloc/malloc.h> everywhere. The only operating system where I could find that header file is MacOS. This is definitely not standard.

You seem to be very dependent on the function malloc_size in many places. As far as I know, only MacOS implements it.

You use the function valloc. This is a non-standard function that has been marked as obsolete in 4.3 BSD in 1986. You can get this functionality from a standard (not C, but at least POSIX) by using the posix_memalign function.

Problematic behavior

In cstring_cmp and cstringbuffer_cmp you compare with the shortest string. Why not just use memcmp? Especially since what you're comparing aren't normal strings because they are not 0円-terminated.

It's possible to write 0円 into a cstringbuffer with cstringbuffer_write_char (and other functions) that will update the length to include that and later append further characters at the end of the buffers, but you're then using normal strcmp, strncpy, etc. to compare and copy the strings which will not do what you want. Either disallow 0円 explicitly or use functions that don't get confused by it.

allocate_mem has a good prototype with count and size. This is just like calloc and it's what a modern memory allocator should do. But, you miss the point of why calloc does it. The main reason is that you can detect overflows when multiplying count * size. You don't check for overflow in that multiplication which can lead to security issues.

Matters of taste

You're always allocating a struct on the stack, then malloc the same struct, then return a copy. Why? It seems like extra work.

Macros with magic goto error; with the error: label being outside of the macro are horrifying.

You do do { ... } while (0) in macros, this is good style. But you then defeat half the purpose of building macros this way by adding the ; inside the macro. for example if (foo) log_error("foo"); else do_something(); will not compile.

Standards

Note also that this project is developed to be compliant with the C11 standard.

Cool, let's find non-standard behavior.

The first thing that jumps out to me is your usage of double underscores.

Both C99 and C11 say (in section 7.1.3):

All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use.

You include <malloc/malloc.h> everywhere. The only operating system where I could find that header file is MacOS. This is definitely not standard.

You seem to be very dependent on the function malloc_size in many places. As far as I know, only MacOS implements it.

You use the function valloc. This is a non-standard function that has been marked as obsolete in 4.3 BSD in 1986. You can get this functionality from a standard (not C, but at least POSIX) by using the posix_memalign function.

Problematic behavior

In cstring_cmp and cstringbuffer_cmp you compare with the shortest string with strncmp which means that you're only comparing the prefixes of the strings (I think, I haven't dug into the exact details of the library) and why not just use memcmp since you're not comparing C strings anyway.

It's possible to write 0円 into a cstringbuffer with cstringbuffer_write_char (and other functions) that will update the length to include that and later append further characters at the end of the buffers, but you're then using normal strcmp, strncpy, etc. to compare and copy the strings which will not do what you want. Either disallow 0円 explicitly or use functions that don't get confused by it.

allocate_mem has a good prototype with count and size. This is just like calloc and it's what a modern memory allocator should do. But, you miss the point of why calloc does it. The main reason is that you can detect overflows when multiplying count * size. You don't check for overflow in that multiplication which can lead to security issues.

Matters of taste

You're always allocating a struct on the stack, then malloc the same struct, then return a copy. Why? It seems like extra work.

Macros with magic goto error; with the error: label being outside of the macro are horrifying.

You do do { ... } while (0) in macros, this is good style. But you then defeat half the purpose of building macros this way by adding the ; inside the macro. for example if (foo) log_error("foo"); else do_something(); will not compile.

some more
Source Link
Art
  • 186
  • 3

Standards

Note also that this project is developed to be compliant with the C11 standard.

Cool, let's find non-standard behavior.

The first thing that jumps out to me is your usage of double underscores.

Both C99 and C11 say (in section 7.1.3):

All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use.

You include <malloc/malloc.h> everywhere. The only operating system where I could find that header file is MacOS. This is definitely not standard.

You seem to be very dependent on the function malloc_size in many places. As far as I know, only MacOS implements it.

You use the function valloc. This is a non-standard function that has been marked as obsolete in 4.3 BSD in 1986. You can get this functionality from a standard (not C, but at least POSIX) by using the posix_memalign function.

Problematic behavior

In cstring_cmp and cstringbuffer_cmp you compare with the shortest string. Why not just use memcmp? Especially since what you're comparing aren't normal strings because they are not 0円-terminated.

It's possible to write 0円 into a cstringbuffer with cstringbuffer_write_char (and other functions) that will update the length to include that and later append further characters at the end of the buffers, but you're then using normal strcmp, strncpy, etc. to compare and copy the strings which will not do what you want. Either disallow 0円 explicitly or use functions that don't get confused by it.

allocate_mem has a good prototype with count and size. This is just like calloc and it's what a modern memory allocator should do. But, you miss the point of why calloc does it. The main reason is that you can detect overflows when multiplying count * size. You don't check for overflow in that multiplication which can lead to security issues.

Matters of taste

You're always allocating a struct on the stack, then malloc the same struct, then return a copy. Why? It seems like extra work.

Macros with magic goto error; with the error: label being outside of the macro are horrifying.

You do do { ... } while (0) in macros, this is good style. But you then defeat half the purpose of building macros this way by adding the ; inside the macro. for example if (foo) log_error("foo"); else do_something(); will not compile.

Standards

Note also that this project is developed to be compliant with the C11 standard.

Cool, let's find non-standard behavior.

The first thing that jumps out to me is your usage of double underscores.

Both C99 and C11 say (in section 7.1.3):

All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use.

You include <malloc/malloc.h> everywhere. The only operating system where I could find that header file is MacOS. This is definitely not standard.

You seem to be very dependent on the function malloc_size in many places. As far as I know, only MacOS implements it.

Problematic behavior

In cstring_cmp and cstringbuffer_cmp you compare with the shortest string. Why not just use memcmp? Especially since what you're comparing aren't normal strings because they are not 0円-terminated.

It's possible to write 0円 into a cstringbuffer with cstringbuffer_write_char (and other functions) that will update the length to include that and later append further characters at the end of the buffers, but you're then using normal strcmp, strncpy, etc. to compare and copy the strings which will not do what you want. Either disallow 0円 explicitly or use functions that don't get confused by it.

Matters of taste

You're always allocating a struct on the stack, then malloc the same struct, then return a copy. Why? It seems like extra work.

Macros with magic goto error; with the error: label being outside of the macro are horrifying.

You do do { ... } while (0) in macros, this is good style. But you then defeat half the purpose of building macros this way by adding the ; inside the macro. for example if (foo) log_error("foo"); else do_something(); will not compile.

Standards

Note also that this project is developed to be compliant with the C11 standard.

Cool, let's find non-standard behavior.

The first thing that jumps out to me is your usage of double underscores.

Both C99 and C11 say (in section 7.1.3):

All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use.

You include <malloc/malloc.h> everywhere. The only operating system where I could find that header file is MacOS. This is definitely not standard.

You seem to be very dependent on the function malloc_size in many places. As far as I know, only MacOS implements it.

You use the function valloc. This is a non-standard function that has been marked as obsolete in 4.3 BSD in 1986. You can get this functionality from a standard (not C, but at least POSIX) by using the posix_memalign function.

Problematic behavior

In cstring_cmp and cstringbuffer_cmp you compare with the shortest string. Why not just use memcmp? Especially since what you're comparing aren't normal strings because they are not 0円-terminated.

It's possible to write 0円 into a cstringbuffer with cstringbuffer_write_char (and other functions) that will update the length to include that and later append further characters at the end of the buffers, but you're then using normal strcmp, strncpy, etc. to compare and copy the strings which will not do what you want. Either disallow 0円 explicitly or use functions that don't get confused by it.

allocate_mem has a good prototype with count and size. This is just like calloc and it's what a modern memory allocator should do. But, you miss the point of why calloc does it. The main reason is that you can detect overflows when multiplying count * size. You don't check for overflow in that multiplication which can lead to security issues.

Matters of taste

You're always allocating a struct on the stack, then malloc the same struct, then return a copy. Why? It seems like extra work.

Macros with magic goto error; with the error: label being outside of the macro are horrifying.

You do do { ... } while (0) in macros, this is good style. But you then defeat half the purpose of building macros this way by adding the ; inside the macro. for example if (foo) log_error("foo"); else do_something(); will not compile.

Source Link
Art
  • 186
  • 3

Standards

Note also that this project is developed to be compliant with the C11 standard.

Cool, let's find non-standard behavior.

The first thing that jumps out to me is your usage of double underscores.

Both C99 and C11 say (in section 7.1.3):

All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use.

You include <malloc/malloc.h> everywhere. The only operating system where I could find that header file is MacOS. This is definitely not standard.

You seem to be very dependent on the function malloc_size in many places. As far as I know, only MacOS implements it.

Problematic behavior

In cstring_cmp and cstringbuffer_cmp you compare with the shortest string. Why not just use memcmp? Especially since what you're comparing aren't normal strings because they are not 0円-terminated.

It's possible to write 0円 into a cstringbuffer with cstringbuffer_write_char (and other functions) that will update the length to include that and later append further characters at the end of the buffers, but you're then using normal strcmp, strncpy, etc. to compare and copy the strings which will not do what you want. Either disallow 0円 explicitly or use functions that don't get confused by it.

Matters of taste

You're always allocating a struct on the stack, then malloc the same struct, then return a copy. Why? It seems like extra work.

Macros with magic goto error; with the error: label being outside of the macro are horrifying.

You do do { ... } while (0) in macros, this is good style. But you then defeat half the purpose of building macros this way by adding the ; inside the macro. for example if (foo) log_error("foo"); else do_something(); will not compile.

lang-c

AltStyle によって変換されたページ (->オリジナル) /