Comments
The count_digits
function could need an explanation (what is its purpose?).
A lot of your comments describe what the code does. This is redundant, I can read the code to see what your code is doing. Instead, a comment should describe why something is done or why something is done in this specific way.
For example, the comment "If this is the last char in the input string, increase the count" is not very helpful. I can see that the code is doing that, but the question is: why is count increased only for the last character?
malloc
Don't cast the malloc
return type. Don't cast the malloc
return type..
Also, why malloc((idx + 1) + 1)
? Why not +2
, and what's the reason for adding 2 in the first place? It needs a comment.
Logic
You're not handling an empty string correctly. You'll allocate 3 bytes instead of 1 byte and won't assign the 0 termination, which might lead to crashes or at least funny garbage when printing.
Return value
In case something is wrong (like invalid input string 123
) you return a copy of the original string. But this is likely to be invalid or nonsense input for your decompression. So you can't distinguish between a valid and invalid return value. I would expect to have NULL
returned on error and errno
set to a reasonable value like EINVAL
.
Standard conformance
strlcpy
is a BSD function and probably not available everywhere. Since you've explicitly listed "compability" and only mentioned C99, you might want to use strncpy
instead. But be careful to correctly append the 0 termination (see also the problem with the empty string).
Comments
The count_digits
function could need an explanation (what is its purpose?).
A lot of your comments describe what the code does. This is redundant, I can read the code to see what your code is doing. Instead, a comment should describe why something is done or why something is done in this specific way.
For example, the comment "If this is the last char in the input string, increase the count" is not very helpful. I can see that the code is doing that, but the question is: why is count increased only for the last character?
malloc
Don't cast the malloc
return type..
Also, why malloc((idx + 1) + 1)
? Why not +2
, and what's the reason for adding 2 in the first place? It needs a comment.
Logic
You're not handling an empty string correctly. You'll allocate 3 bytes instead of 1 byte and won't assign the 0 termination, which might lead to crashes or at least funny garbage when printing.
Return value
In case something is wrong (like invalid input string 123
) you return a copy of the original string. But this is likely to be invalid or nonsense input for your decompression. So you can't distinguish between a valid and invalid return value. I would expect to have NULL
returned on error and errno
set to a reasonable value like EINVAL
.
Standard conformance
strlcpy
is a BSD function and probably not available everywhere. Since you've explicitly listed "compability" and only mentioned C99, you might want to use strncpy
instead. But be careful to correctly append the 0 termination (see also the problem with the empty string).
Comments
The count_digits
function could need an explanation (what is its purpose?).
A lot of your comments describe what the code does. This is redundant, I can read the code to see what your code is doing. Instead, a comment should describe why something is done or why something is done in this specific way.
For example, the comment "If this is the last char in the input string, increase the count" is not very helpful. I can see that the code is doing that, but the question is: why is count increased only for the last character?
malloc
Don't cast the malloc
return type..
Also, why malloc((idx + 1) + 1)
? Why not +2
, and what's the reason for adding 2 in the first place? It needs a comment.
Logic
You're not handling an empty string correctly. You'll allocate 3 bytes instead of 1 byte and won't assign the 0 termination, which might lead to crashes or at least funny garbage when printing.
Return value
In case something is wrong (like invalid input string 123
) you return a copy of the original string. But this is likely to be invalid or nonsense input for your decompression. So you can't distinguish between a valid and invalid return value. I would expect to have NULL
returned on error and errno
set to a reasonable value like EINVAL
.
Standard conformance
strlcpy
is a BSD function and probably not available everywhere. Since you've explicitly listed "compability" and only mentioned C99, you might want to use strncpy
instead. But be careful to correctly append the 0 termination (see also the problem with the empty string).
Comments
The count_digits
function could need an explanation (what is its purpose?).
A lot of your comments describe what the code does. This is redundant, I can read the code to see what your code is doing. Instead, a comment should describe why something is done or why something is done in this specific way.
For example, the comment "If this is the last char in the input string, increase the count" is not very helpful. I can see that the code is doing that, but the question is: why is count increased only for the last character?
malloc
Don't cast the malloc
return type..
Also, why malloc((idx + 1) + 1)
? Why not +2
, and what's the reason for adding 2 in the first place? It needs a comment.
Logic
You're not handling an empty string correctly. You'll allocate 3 bytes instead of 1 bytesbyte and won't assign the 0 termination, which might lead to crashes or at least funny garbage when printing.
Return value
In case something is wrong (like invalid input string 123
) you return a copy of the original string. But this is likely to be invalid or nonsense input for your decompression. So you can't distinguish between a valid and invalid return value. I would expect to have NULL
returned on error and errno
set to a reasonable value like EINVAL
.
Standard conformance
strlcpy
is a BSD function and probably not available everywhere. Since you've explicitly listed "compability" and only mentioned C99, you might want to use strncpy
instead. But be careful to correctly append the 0 termination (see also the problem with the empty string).
Comments
The count_digits
function could need an explanation (what is its purpose?).
A lot of your comments describe what the code does. This is redundant, I can read the code to see what your code is doing. Instead, a comment should describe why something is done or why something is done in this specific way.
For example, the comment "If this is the last char in the input string, increase the count" is not very helpful. I can see that the code is doing that, but the question is: why is count increased only for the last character?
malloc
Don't cast the malloc
return type..
Also, why malloc((idx + 1) + 1)
? Why not +2
, and what's the reason for adding 2 in the first place? It needs a comment.
Logic
You're not handling an empty string correctly. You'll allocate 3 instead of 1 bytes and won't assign the 0 termination.
Return value
In case something is wrong (like invalid input string 123
) you return a copy of the original string. But this is likely to be invalid or nonsense input for your decompression. So you can't distinguish between a valid and invalid return value. I would expect to have NULL
returned on error and errno
set to a reasonable value like EINVAL
.
Standard conformance
strlcpy
is a BSD function and probably not available everywhere. Since you've explicitly listed "compability" and only mentioned C99, you might want to use strncpy
instead. But be careful to correctly append the 0 termination (see also the problem with the empty string).
Comments
The count_digits
function could need an explanation (what is its purpose?).
A lot of your comments describe what the code does. This is redundant, I can read the code to see what your code is doing. Instead, a comment should describe why something is done or why something is done in this specific way.
For example, the comment "If this is the last char in the input string, increase the count" is not very helpful. I can see that the code is doing that, but the question is: why is count increased only for the last character?
malloc
Don't cast the malloc
return type..
Also, why malloc((idx + 1) + 1)
? Why not +2
, and what's the reason for adding 2 in the first place? It needs a comment.
Logic
You're not handling an empty string correctly. You'll allocate 3 bytes instead of 1 byte and won't assign the 0 termination, which might lead to crashes or at least funny garbage when printing.
Return value
In case something is wrong (like invalid input string 123
) you return a copy of the original string. But this is likely to be invalid or nonsense input for your decompression. So you can't distinguish between a valid and invalid return value. I would expect to have NULL
returned on error and errno
set to a reasonable value like EINVAL
.
Standard conformance
strlcpy
is a BSD function and probably not available everywhere. Since you've explicitly listed "compability" and only mentioned C99, you might want to use strncpy
instead. But be careful to correctly append the 0 termination (see also the problem with the empty string).
Comments
The count_digits
function could need an explanation (what is its purpose?).
A lot of your comments describe what the code does. This is redundant, I can read the code to see what your code is doing. Instead, a comment should describe why something is done or why something is done in this specific way.
For example, the comment "If this is the last char in the input string, increase the count" is not very helpful. I can see that the code is doing that, but the question is: why is count increased only for the last character?
malloc
Don't cast the malloc
return type..
Also, why malloc((idx + 1) + 1)
? Why not +2
, and what's the reason for adding 2 in the first place? It needs a comment.
Logic
You're not handling an empty string correctly. You'll allocate 3 instead of 1 bytes and won't assign the 0 termination.
Return value
In case something is wrong (like invalid input string 123
) you return a copy of the original string. But this is likely to be invalid or nonsense input for your decompression. So you can't distinguish between a valid and invalid return value. I would expect to have NULL
returned on error and errno
set to a reasonable value like EINVAL
.
Standard conformance
strlcpy
is a BSD function and probably not available everywhere. Since you've explicitly listed "compability" and only mentioned C99, you might want to use strncpy
instead. But be careful to correctly append the 0 termination (see also the problem with the empty string).