alelievr / libft-unit-test

453 stars 88 forks source link

false-positives #151

Closed hamza-cskn closed 1 year ago

hamza-cskn commented 1 year ago

i know sometimes people could not see their mistakes even they checked out carefully. if i am one of them, sorry. however i read the test code and i could not reproduce same issue. when i add a debug printf code to the the test code, it says 'success', but it still assumes failed.

split protect issue #119

i have same issue. i debugged my code. however i couldn't find any problem like you've(@Rodeo314) mentioned. it always allocates exactly amount of necessary bytes. and ensures the string is null-terminated.

my split code: https://paste.ajg0702.us/PMVcFiQ

atoi

my atoi code returns same as original atoi. and the test function agrees that but it fails still. i modified the original test code and just added success printf code.

modified test code: https://paste.ajg0702.us/dudCuE4 test result:

atoi test failed

my atoi code: https://paste.ajg0702.us/S5JVaCb

memmove crash

modified memove test code: https://paste.ajg0702.us/KTO6hgK test result:

memmove crash

my memmove code: https://paste.ajg0702.us/HsVJdAn

you don't have to review/answer for all issues at one time. thank you for interesting.

github-actions[bot] commented 1 year ago

Hello! Thanks for contributing to the libft unit test.

Note that this repository is not maintained by the owner anymore, instead there is a bot that will automatically merge any reviewed pull requests. If you feel like it, here are some links that can help you submiting a change in the code base::

ghost commented 1 year ago

~It would be helpful to show the exact results of the test including the error message, instead of just your code.~

My bad, I was thinking of another issue. Your report does show the errors messages.

ghost commented 1 year ago

Your ft_memmove:

copy(-1, len + 1, dst, src);

start is -1 and i becomes start, first iteration, dst[i] = src[i] is dst[-1] = src[-1] out of bounds

You want to start at 0, not -1, and you also need to update the condition for the increment selection in copy().

Tester is correctly pointing out an error, the message is misleading because your mistake is probably not typical for this kind of function.

ghost commented 1 year ago

ft_atoi: the error message is completely wrong (plus sign and spaces are not the problem), the tester is actually checking ft_atoi(NULL) and expecting a crash, your ft_atoi does not crash.

https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf

7.1.4 Use of library functions 1 Each of the following statements applies unless explicitly stated otherwise in the detailed descriptions that follow: If an argument to a function has an invalid value (such as a value outside the domain of the function, or a pointer outside the address space of the program, or a null pointer, or a pointer to non-modifiable storage when the corresponding parameter is not const-qualified) or a type (after promotion) not expected by a function with variable number of arguments, the behavior is undefined.

The detailed description of atoi (in section 7.20.1.2) does not state that atoi may accept a null pointer, thus atoi(NULL) is arguably undefined behavior (when str is NULL, crashing or not crashing would be equally correct); the test is a bit pointless. But crashing would be easy, just don't check for NULL input.

ghost commented 1 year ago

In your ft_split, word_length:

while (str[i] != c)
    i++;

When checking the length of the last word, eventually str[i] will be == 0, you are not checking str[i] and the length will be incorrect for the last word, you need to while (str[i] && str[i] != c) there too…

result = malloc(sizeof(char *) * (word_amount + 1));

…is not protected at all.

res = malloc(sizeof(char) * size);

…is not protected properly, because you placed your debug memset before the protection.

Your debug memsets are also buggy, because of the fixed length of 100 you may write past the end of the allocated memory, causing other issues.

hamza-cskn commented 1 year ago

Thank you. I fixed my memmove and atoi. They passes all test now 👍. However i could not fixed my ft_split. I added str[i] check to my word length function. I checked out again the number of bytes allocated.

My latest ft_split code: https://pastes.dev/T1yruesF9n

hamza-cskn commented 1 year ago

i forgot to add

res = malloc(...)
if (!res)
    return NULL;

to my code. after that, everything is done.