Some things come to mind when looking through the source. Perhaps they are pedantic gripes, but perhaps they need some work. I had these observations while looking through memcpy.c:
1. Assumption of power of 2 word size. The ALIGN macro is defined as one less than the size of size_t (which should be enough to hold the length of something in memory). I would be more comfortable with it being defined as the the size of a processor WORD rather than as a length of memory.
2. Using the ALIGN macro in calculations of misaligned memory copies. This goes to #1's point: Suppose the word size is not a power of 2? Suppose the word size is 6 octets rather than 4 or 8. The & mask might produce incorrect results since there would be 0's in the mask.
3. There seems to be a lot of casting going on where it either shouldn't be or it looks wrong. A good example of this is the cast to void on line 19. If there is a cast, why isn't to the (size_t*) type? Even if this is correct, I now have to think a lot more about it, or hunt through the commit log. Yes, I could. No, I shouldn't have to.
4. The ONES macro is unused, and the ALIGN MACRO are not part of a header somewhere. An alignment macro seems like it might be beneficial in other parts of the library. I recognize that this increases interdependency, but I feel it may be justified since it would be useful in other place, and may be changed in the future.
I am very pleased that this library is being developed. I think competition drives up the quality of code, and libc is one of the places where high quality code is invaluable. (Looking at the comparison chart they provided was impressive!) I must admit I don't have good solution to the issues I found, so I'm afraid my criticism may not be as constructive as I would prefer.
Thanks for making this, it was quite pleasant to look at non Drepper'd libc code. :)
I agree that it's conceptually nice to support non-power of two word sizes, but how often do those come up these days in practice? If none of the architectures they're targeting actually have such a word size, it seems like it might be a useful simplifying assumption.
I would agree in full, those oddball machines don't come up very often. But when they do, its really nice to have something that makes as few assumptions about the underlying hardware as possible. If I recall correctly C89 doesn't make any requirements about size (except that sizeof(char) is always 1).
It is rare that libraries get rewritten from the ground up, so this would be the perfect time to get the code right.
1. Assumption of power of 2 word size. The ALIGN macro is defined as one less than the size of size_t (which should be enough to hold the length of something in memory). I would be more comfortable with it being defined as the the size of a processor WORD rather than as a length of memory.
2. Using the ALIGN macro in calculations of misaligned memory copies. This goes to #1's point: Suppose the word size is not a power of 2? Suppose the word size is 6 octets rather than 4 or 8. The & mask might produce incorrect results since there would be 0's in the mask.
3. There seems to be a lot of casting going on where it either shouldn't be or it looks wrong. A good example of this is the cast to void on line 19. If there is a cast, why isn't to the (size_t*) type? Even if this is correct, I now have to think a lot more about it, or hunt through the commit log. Yes, I could. No, I shouldn't have to.
4. The ONES macro is unused, and the ALIGN MACRO are not part of a header somewhere. An alignment macro seems like it might be beneficial in other parts of the library. I recognize that this increases interdependency, but I feel it may be justified since it would be useful in other place, and may be changed in the future.
I am very pleased that this library is being developed. I think competition drives up the quality of code, and libc is one of the places where high quality code is invaluable. (Looking at the comparison chart they provided was impressive!) I must admit I don't have good solution to the issues I found, so I'm afraid my criticism may not be as constructive as I would prefer.
Thanks for making this, it was quite pleasant to look at non Drepper'd libc code. :)
Here is the link I looked at for reference: http://git.etalabs.net/cgi-bin/gitweb.cgi?p=musl;a=blob;f=sr...