Skip to main content
added 77 characters in body
Source Link
Deduplicator
  • 19.3k
  • 1
  • 31
  • 65

Documentation:

Be more definite in your descriptions. Does the null pointer returned on requesting zero bytes indicate a failure?

In that case, shorter and unambiguous: "Returns a pointer to the allocated space or a null pointer on failure. Requesting zero bytes always fails."

In the other case, consider this: "Returns a pointer to the allocated space or a null pointer. Requesting zero bytes always succeeds with a null pointer."

Admittedly, that only really gets important when we get to realloc, but it should be consistent throughout: "If *size* is zero and memory for the new object is not allocated, the old object is not deallocated, and its value shall be unchanged."
What? Can we allocate zero bytes? Can/Will that result in a null pointer? Will that indicate success, failure, or Schrödinger?

Also, can shrinking fail? Will it ever free memory?

As the documentation should stand alone, I have ignored the implementation to this point.


Tests

Are very much missing.
No wonder the next section has big items.

Implementation bug:

  1. You are off by onethe padding in your implementation of malloc(). A size of size <= SIZE_MAX - sizeof(header_t) willstill allows size + padding > SIZE_MAX - sizeof(header_t), causing you to pass 0 to sbrk() and assume it got SIZE_MAX + 1 bytes, though the function just returned the current value.

  2. Did you try out calloc?
    Your second check goes in the wrong direction.

Implementation:

  1. Using macros for the locking allows easy removal of same, so can be justified.
    It allows for easy adoption of the same algorithm for single-threaded and low-contention multi-threaded use.

  2. The rest of those macros, used for operations on chunks, just obscure the code though. Best to get rid of the lot.
    Point of fact, I gave up digging into free and thus overlooked another bug Toby Speight found.

  3. Consider mmap for private anonymous pages (and mremap for realloc) instead of brk for additional memory. It's the modern approach for good reason, namely playing nicely with ASLR, and still working if there is something in the way of a straight expansion.

    Admittedly, as it works with page-size granularity, it makes invest ing into splitting and coalescing blocks that much more urgent.

  4. If you free the last chunk requested from the OS you haven't yet returned, you return it. But all the free chunks next to it will be kept...

  5. Consider a uniform limit on the line length, preferably equal to the customary 80.
    Vertical scrolling is bad enough for comprehension, but horizontal scrolling is orders of magnitude worse.
    The makefile already conforms, the implementation file is wrapped just above, but the header file is wrapped significantly later...

Documentation:

Be more definite in your descriptions. Does the null pointer returned on requesting zero bytes indicate a failure?

In that case, shorter and unambiguous: "Returns a pointer to the allocated space or a null pointer on failure. Requesting zero bytes always fails."

In the other case, consider this: "Returns a pointer to the allocated space or a null pointer. Requesting zero bytes always succeeds with a null pointer."

Admittedly, that only really gets important when we get to realloc, but it should be consistent throughout: "If *size* is zero and memory for the new object is not allocated, the old object is not deallocated, and its value shall be unchanged."
What? Can we allocate zero bytes? Can/Will that result in a null pointer? Will that indicate success, failure, or Schrödinger?

Also, can shrinking fail? Will it ever free memory?

As the documentation should stand alone, I have ignored the implementation to this point.


Tests

Are very much missing.
No wonder the next section has big items.

Implementation bug:

  1. You are off by one in your implementation of malloc(). A size of SIZE_MAX - sizeof(header_t) will pass 0 to sbrk() and assume it got SIZE_MAX + 1 bytes, though the function just returned the current value.

  2. Did you try out calloc?
    Your second check goes in the wrong direction.

Implementation:

  1. Using macros for the locking allows easy removal of same, so can be justified.
    It allows for easy adoption of the same algorithm for single-threaded and low-contention multi-threaded use.

  2. The rest of those macros, used for operations on chunks, just obscure the code though. Best to get rid of the lot.
    Point of fact, I gave up digging into free and thus overlooked another bug Toby Speight found.

  3. Consider mmap for private anonymous pages (and mremap for realloc) instead of brk for additional memory. It's the modern approach for good reason, namely playing nicely with ASLR, and still working if there is something in the way of a straight expansion.

    Admittedly, as it works with page-size granularity, it makes invest ing into splitting and coalescing blocks that much more urgent.

  4. If you free the last chunk requested from the OS you haven't yet returned, you return it. But all the free chunks next to it will be kept...

  5. Consider a uniform limit on the line length, preferably equal to the customary 80.
    Vertical scrolling is bad enough for comprehension, but horizontal scrolling is orders of magnitude worse.
    The makefile already conforms, the implementation file is wrapped just above, but the header file is wrapped significantly later...

Documentation:

Be more definite in your descriptions. Does the null pointer returned on requesting zero bytes indicate a failure?

In that case, shorter and unambiguous: "Returns a pointer to the allocated space or a null pointer on failure. Requesting zero bytes always fails."

In the other case, consider this: "Returns a pointer to the allocated space or a null pointer. Requesting zero bytes always succeeds with a null pointer."

Admittedly, that only really gets important when we get to realloc, but it should be consistent throughout: "If *size* is zero and memory for the new object is not allocated, the old object is not deallocated, and its value shall be unchanged."
What? Can we allocate zero bytes? Can/Will that result in a null pointer? Will that indicate success, failure, or Schrödinger?

Also, can shrinking fail? Will it ever free memory?

As the documentation should stand alone, I have ignored the implementation to this point.


Tests

Are very much missing.
No wonder the next section has big items.

Implementation bug:

  1. You are off by the padding in your implementation of malloc(). size <= SIZE_MAX - sizeof(header_t) still allows size + padding > SIZE_MAX - sizeof(header_t), causing you to pass 0 to sbrk() and assume it got SIZE_MAX + 1 bytes, though the function just returned the current value.

  2. Did you try out calloc?
    Your second check goes in the wrong direction.

Implementation:

  1. Using macros for the locking allows easy removal of same, so can be justified.
    It allows for easy adoption of the same algorithm for single-threaded and low-contention multi-threaded use.

  2. The rest of those macros, used for operations on chunks, just obscure the code though. Best to get rid of the lot.
    Point of fact, I gave up digging into free and thus overlooked another bug Toby Speight found.

  3. Consider mmap for private anonymous pages (and mremap for realloc) instead of brk for additional memory. It's the modern approach for good reason, namely playing nicely with ASLR, and still working if there is something in the way of a straight expansion.

    Admittedly, as it works with page-size granularity, it makes invest ing into splitting and coalescing blocks that much more urgent.

  4. If you free the last chunk requested from the OS you haven't yet returned, you return it. But all the free chunks next to it will be kept...

  5. Consider a uniform limit on the line length, preferably equal to the customary 80.
    Vertical scrolling is bad enough for comprehension, but horizontal scrolling is orders of magnitude worse.
    The makefile already conforms, the implementation file is wrapped just above, but the header file is wrapped significantly later...

added 84 characters in body
Source Link
Deduplicator
  • 19.3k
  • 1
  • 31
  • 65

Documentation:

Be more definite in your descriptions. Does the null pointer returned on requesting zero bytes indicate a failure?

In that case, shorter and unambiguous: "Returns a pointer to the allocated space or a null pointer on failure. Requesting zero bytes always fails."

In the other case, consider this: "Returns a pointer to the allocated space or a null pointer. Requesting zero bytes always succeeds with a null pointer."

Admittedly, that only really gets important when we get to realloc, but it should be consistent throughout: "If *size* is zero and memory for the new object is not allocated, the old object is not deallocated, and its value shall be unchanged."
What? Can we allocate zero bytes? Can/Will that result in a null pointer? Will that indicate success, failure, or Schrödinger?

Also, can shrinking fail? Will it ever free memory?

As the documentation should stand alone, I have ignored the implementation to this point.


Tests

Are very much missing.
No wonder the next section has big items.

Implementation bug:

  1. You are off by one in your implementation of malloc(). A size of SIZE_MAX - sizeof(header_t) will pass 0 to sbrk() and assume it got SIZE_MAX + 1 bytes, though the function just returned the current value.

  2. Did you try out calloc?
    Your second check goes in the wrong direction.

Implementation:

  1. Using macros for the locking allows easy removal of same, so can be justified.
    It allows for easy adoption of the same algorithm for single-threaded and low-contention multi-threaded use.

  2. The rest of those macros, used for operations on chunks, just obscure the code though. Best to get rid of the lot.
    Point of fact, I gave up digging into free and thus overlooked another bug Toby Speight found.

  3. Consider mmap for private anonymous pages (and mremap for realloc) instead of brk for additional memory. It's the modern approach for good reason, namely playing nicely with ASLR, and still working if there is something in the way of a straight expansion.

    Admittedly, as it works with page-size granularity, it makes invest ing into splitting and coalescing blocks that much more urgent.

  4. If you free the last chunk requested from the OS you haven't yet returned, you return it. But all the free chunks next to it will be kept...

  5. Consider a uniform limit on the line length, preferably equal to the customary 80.
    Vertical scrolling is bad enough for comprehension, but horizontal scrolling is orders of magnitude worse.
    The makefile already conforms, the implementation file is wrapped just above, but the header file is wrapped significantly later...

Documentation:

Be more definite in your descriptions. Does the null pointer returned on requesting zero bytes indicate a failure?

In that case, shorter and unambiguous: "Returns a pointer to the allocated space or a null pointer on failure. Requesting zero bytes always fails."

In the other case, consider this: "Returns a pointer to the allocated space or a null pointer. Requesting zero bytes always succeeds with a null pointer."

Admittedly, that only really gets important when we get to realloc, but it should be consistent throughout: "If *size* is zero and memory for the new object is not allocated, the old object is not deallocated, and its value shall be unchanged."
What? Can we allocate zero bytes? Can/Will that result in a null pointer? Will that indicate success, failure, or Schrödinger?

Also, can shrinking fail? Will it ever free memory?

As the documentation should stand alone, I have ignored the implementation to this point.


Implementation bug:

  1. You are off by one in your implementation of malloc(). A size of SIZE_MAX - sizeof(header_t) will pass 0 to sbrk() and assume it got SIZE_MAX + 1 bytes, though the function just returned the current value.

  2. Did you try out calloc?
    Your second check goes in the wrong direction.

Implementation:

  1. Using macros for the locking allows easy removal of same, so can be justified.
    It allows for easy adoption of the same algorithm for single-threaded and low-contention multi-threaded use.

  2. The rest of those macros, used for operations on chunks, just obscure the code though. Best to get rid of the lot.
    Point of fact, I gave up digging into free and thus overlooked another bug Toby Speight found.

  3. Consider mmap for private anonymous pages (and mremap for realloc) instead of brk for additional memory. It's the modern approach for good reason, namely playing nicely with ASLR, and still working if there is something in the way of a straight expansion.

    Admittedly, as it works with page-size granularity, it makes invest ing into splitting and coalescing blocks that much more urgent.

  4. If you free the last chunk requested from the OS you haven't yet returned, you return it. But all the free chunks next to it will be kept...

  5. Consider a uniform limit on the line length, preferably equal to the customary 80.
    Vertical scrolling is bad enough for comprehension, but horizontal scrolling is orders of magnitude worse.
    The makefile already conforms, the implementation file is wrapped just above, but the header file is wrapped significantly later...

Documentation:

Be more definite in your descriptions. Does the null pointer returned on requesting zero bytes indicate a failure?

In that case, shorter and unambiguous: "Returns a pointer to the allocated space or a null pointer on failure. Requesting zero bytes always fails."

In the other case, consider this: "Returns a pointer to the allocated space or a null pointer. Requesting zero bytes always succeeds with a null pointer."

Admittedly, that only really gets important when we get to realloc, but it should be consistent throughout: "If *size* is zero and memory for the new object is not allocated, the old object is not deallocated, and its value shall be unchanged."
What? Can we allocate zero bytes? Can/Will that result in a null pointer? Will that indicate success, failure, or Schrödinger?

Also, can shrinking fail? Will it ever free memory?

As the documentation should stand alone, I have ignored the implementation to this point.


Tests

Are very much missing.
No wonder the next section has big items.

Implementation bug:

  1. You are off by one in your implementation of malloc(). A size of SIZE_MAX - sizeof(header_t) will pass 0 to sbrk() and assume it got SIZE_MAX + 1 bytes, though the function just returned the current value.

  2. Did you try out calloc?
    Your second check goes in the wrong direction.

Implementation:

  1. Using macros for the locking allows easy removal of same, so can be justified.
    It allows for easy adoption of the same algorithm for single-threaded and low-contention multi-threaded use.

  2. The rest of those macros, used for operations on chunks, just obscure the code though. Best to get rid of the lot.
    Point of fact, I gave up digging into free and thus overlooked another bug Toby Speight found.

  3. Consider mmap for private anonymous pages (and mremap for realloc) instead of brk for additional memory. It's the modern approach for good reason, namely playing nicely with ASLR, and still working if there is something in the way of a straight expansion.

    Admittedly, as it works with page-size granularity, it makes invest ing into splitting and coalescing blocks that much more urgent.

  4. If you free the last chunk requested from the OS you haven't yet returned, you return it. But all the free chunks next to it will be kept...

  5. Consider a uniform limit on the line length, preferably equal to the customary 80.
    Vertical scrolling is bad enough for comprehension, but horizontal scrolling is orders of magnitude worse.
    The makefile already conforms, the implementation file is wrapped just above, but the header file is wrapped significantly later...

added 552 characters in body
Source Link
Deduplicator
  • 19.3k
  • 1
  • 31
  • 65

Documentation:

Be more definite in your descriptions. Does the null pointer returned on requesting zero bytes indicate a failure?

In that case, shorter and unambiguous: "Returns a pointer to the allocated space or a null pointer on failure. Requesting zero bytes always fails."

In the other case, consider this: "Returns a pointer to the allocated space or a null pointer. Requesting zero bytes always succeeds with a null pointer."

Admittedly, that only really gets important when we get to realloc, but it should be consistent throughout: "If *size* is zero and memory for the new object is not allocated, the old object is not deallocated, and its value shall be unchanged."
What? Can we allocate zero bytes? Can/Will that result in a null pointer? Will that indicate success, failure, or Schrödinger?

Also, can shrinking fail? Will it ever free memory?

As the documentation should stand alone, I have ignored the implementation to this point.


Implementation bug:

  1. You are off by one in your implementation of malloc(). A size of SIZE_MAX - sizeof(header_t) will pass 0 to sbrk() and assume it got SIZE_MAX + 1 bytes, though the function just returned the current value.

  2. Did you try out calloc?
    Your second check goes in the wrong direction.

Implementation:

  1. Using macros for the locking allows easy removal of same, so can be justified.
    But allIt allows for easy adoption of the same algorithm for single-threaded and low-contention multi-threaded use.

  2. The rest of those macros, used for operations on chunks, just obscure thingsthe code though. Best to get rid of the lot.
    Point of fact, I gave up digging into free and thus overlooked another bug Toby Speight found.

  3. Consider using memory mappingmmap for private anonymous pages (and mremap for realloc) instead of brk for additional memory. It's the modern approach for reasonsgood reason, namely playing nicely with ASLR, and still working if there is something in the way of a straight expansion.

    Admittedly, as it works with page-size granularity, it makes invest ing into splitting and coalescing blocks that much more urgent.

  4. If you free the last chunk requested from the OS you haven't yet returned, you return it. But all the free chunks next to it will be kept...

  5. Consider a uniform limit on the line length, preferably equal to the customary 80.
    Vertical scrolling is bad enough for comprehension, but horizontal scrolling is orders of magnitude worse.
    The makefile already conforms, the implementation file is wrapped just above, but the header file is wrapped significantly later...

Documentation:

Be more definite in your descriptions. Does the null pointer returned on requesting zero bytes indicate a failure?

In that case, shorter and unambiguous: "Returns a pointer to the allocated space or a null pointer on failure. Requesting zero bytes always fails."

In the other case, consider this: "Returns a pointer to the allocated space or a null pointer. Requesting zero bytes always succeeds with a null pointer."

Admittedly, that only really gets important when we get to realloc, but it should be consistent throughout: "If *size* is zero and memory for the new object is not allocated, the old object is not deallocated, and its value shall be unchanged."
What? Can we allocate zero bytes? Can/Will that result in a null pointer? Will that indicate success, failure, or Schrödinger?

Also, can shrinking fail? Will it ever free memory?

As the documentation should stand alone, I have ignored the implementation to this point.


Implementation bug:

  1. You are off by one in your implementation of malloc(). A size of SIZE_MAX - sizeof(header_t) will pass 0 to sbrk() and assume it got SIZE_MAX + 1 bytes, though the function just returned the current value.

  2. Did you try out calloc?
    Your second check goes in the wrong direction.

Implementation:

  1. Using macros for the locking allows easy removal of same, so can be justified.
    But all those macros for operations on chunks just obscure things.

  2. Consider using memory mapping instead of brk for additional memory. It's the modern approach for reasons, namely playing nicely with ASLR.

  3. If you free the last chunk requested from the OS you haven't yet returned, you return it. But all the free chunks next to it will be kept...

  4. Consider a uniform limit on the line length equal to the customary 80.
    The makefile already conforms, the implementation file is wrapped just above, but the header is wrapped significantly later...

Documentation:

Be more definite in your descriptions. Does the null pointer returned on requesting zero bytes indicate a failure?

In that case, shorter and unambiguous: "Returns a pointer to the allocated space or a null pointer on failure. Requesting zero bytes always fails."

In the other case, consider this: "Returns a pointer to the allocated space or a null pointer. Requesting zero bytes always succeeds with a null pointer."

Admittedly, that only really gets important when we get to realloc, but it should be consistent throughout: "If *size* is zero and memory for the new object is not allocated, the old object is not deallocated, and its value shall be unchanged."
What? Can we allocate zero bytes? Can/Will that result in a null pointer? Will that indicate success, failure, or Schrödinger?

Also, can shrinking fail? Will it ever free memory?

As the documentation should stand alone, I have ignored the implementation to this point.


Implementation bug:

  1. You are off by one in your implementation of malloc(). A size of SIZE_MAX - sizeof(header_t) will pass 0 to sbrk() and assume it got SIZE_MAX + 1 bytes, though the function just returned the current value.

  2. Did you try out calloc?
    Your second check goes in the wrong direction.

Implementation:

  1. Using macros for the locking allows easy removal of same, so can be justified.
    It allows for easy adoption of the same algorithm for single-threaded and low-contention multi-threaded use.

  2. The rest of those macros, used for operations on chunks, just obscure the code though. Best to get rid of the lot.
    Point of fact, I gave up digging into free and thus overlooked another bug Toby Speight found.

  3. Consider mmap for private anonymous pages (and mremap for realloc) instead of brk for additional memory. It's the modern approach for good reason, namely playing nicely with ASLR, and still working if there is something in the way of a straight expansion.

    Admittedly, as it works with page-size granularity, it makes invest ing into splitting and coalescing blocks that much more urgent.

  4. If you free the last chunk requested from the OS you haven't yet returned, you return it. But all the free chunks next to it will be kept...

  5. Consider a uniform limit on the line length, preferably equal to the customary 80.
    Vertical scrolling is bad enough for comprehension, but horizontal scrolling is orders of magnitude worse.
    The makefile already conforms, the implementation file is wrapped just above, but the header file is wrapped significantly later...

Source Link
Deduplicator
  • 19.3k
  • 1
  • 31
  • 65
Loading