Skip to main content
added 323 characters in body
Source Link
chux
  • 36.4k
  • 2
  • 43
  • 97

Segment code into a libint128.h and libint128.c and minimumminimize #include<>#include <....h> lines in the .h.h file.

It is zero. Using UINT128_MIN is a C anti-pattern.

Using spaces is potentialpotentially more profitable.

Consistency

Code has isdigit( *p ) and *p >= '0' && *p <= '9'.
Use a consistent style.

Way too many comments

Advanced

Consider that all these function could be written abstractly with just a few defines and type-defs up front.

Then easily to port to various integer types as only that top section needs changing.

Segment code into a libint128.h and libint128.c and minimum #include<> lines in the .h file.

It is zero.

Using spaces is potential more profitable.

Way too many comments

Segment code into a libint128.h and libint128.c and minimize #include <....h> lines in the .h file.

It is zero. Using UINT128_MIN is a C anti-pattern.

Using spaces is potentially more profitable.

Consistency

Code has isdigit( *p ) and *p >= '0' && *p <= '9'.
Use a consistent style.

Way too many comments

Advanced

Consider that all these function could be written abstractly with just a few defines and type-defs up front.

Then easily to port to various integer types as only that top section needs changing.

added 75 characters in body
Source Link
chux
  • 36.4k
  • 2
  • 43
  • 97

.h for declarations, definitions, etc. - not function code nor data

Segment code into a libint128.h and libint128.c and minimum #include<> lines in the .h file.

Strange to test if( !isdigit( *p ) ) { twice in asctou128()

Once is sufficient.

Don't bother defining U128_MIN

It is zero.

// #define U128_MIN (uint128)0
// #define U128_MAX (uint128)( ~U128_MIN )   
#define U128_MAX ((uint128)-1)

Style: Tabs vs. spaces

Using spaces is potential more profitable.

Undefined behavior

is...(some_negative_not_EOF) is undefined behavior. Access via unsigned char * for maximum portability. A char may be negative.

// const char *p = s;
// ... 
// while( isspace( *p ) ) { 

const unsigned char *p = (const unsigned char *) s;
... 
while( isspace( *p ) ) { 

Do not use errno = ERANGE for a non-range issue

if( !isdigit( *p ) ) { 
  errno = ERANGE; // Do not do this.
  // Re-think error indication strategy.

Unneeded cast

// val = (uint128)( 10 * val ) + (uint128)( *p - '0' ); 
val = 10 * val + (uint128)( *p - '0' ); 

Use boolean assignments

neg is used as a boolean flag. Use boolean constants.

// bool neg = 0; 
bool neg = false; 

Detect buffer access

I'd expect u128toasc() to return with a failure indication if the supplied buffer is too small.

I recommend to use an internal buffer of sufficient size for all int128 and then instead of memmove( str, str + j, len - j ), check size requirements (maybe test len > j) and then memcpy(str, local_buffer + j, len - j).

Use 10 for base 10 conversions

// while( x < -9 ) {
while( x <= -10 ) {

Minor: simplify

// memmove( str, str + j, len - j );
// return( str ); 

return memmove( str, str + j, len - j );

Way too many comments

Don't bother defining U128_MIN

It is zero.

// #define U128_MIN (uint128)0
// #define U128_MAX (uint128)( ~U128_MIN )   
#define U128_MAX ((uint128)-1)

Style: Tabs vs. spaces

Using spaces is potential more profitable.

Undefined behavior

is...(some_negative_not_EOF) is undefined behavior. Access via unsigned char * for maximum portability. A char may be negative.

// const char *p = s;
// ... 
// while( isspace( *p ) ) { 

const unsigned char *p = (const unsigned char *) s;
... 
while( isspace( *p ) ) { 

Do not use errno = ERANGE for a non-range issue

if( !isdigit( *p ) ) { 
  errno = ERANGE; // Do not do this.
  // Re-think error indication strategy.

Unneeded cast

// val = (uint128)( 10 * val ) + (uint128)( *p - '0' ); 
val = 10 * val + (uint128)( *p - '0' ); 

Use boolean assignments

neg is used as a boolean flag. Use boolean constants.

// bool neg = 0; 
bool neg = false; 

Detect buffer access

I'd expect u128toasc() to return with a failure indication if the supplied buffer is too small.

I recommend to use an internal buffer of sufficient size for all int128 and then instead of memmove( str, str + j, len - j ), check size requirements (maybe test len > j) and then memcpy(str, local_buffer + j, len - j).

Use 10 for base 10 conversions

// while( x < -9 ) {
while( x <= -10 ) {

Minor: simplify

// memmove( str, str + j, len - j );
// return( str ); 

return memmove( str, str + j, len - j );

.h for declarations, definitions, etc. - not function code nor data

Segment code into a libint128.h and libint128.c and minimum #include<> lines in the .h file.

Strange to test if( !isdigit( *p ) ) { twice in asctou128()

Once is sufficient.

Don't bother defining U128_MIN

It is zero.

// #define U128_MIN (uint128)0
// #define U128_MAX (uint128)( ~U128_MIN )   
#define U128_MAX ((uint128)-1)

Style: Tabs vs. spaces

Using spaces is potential more profitable.

Undefined behavior

is...(some_negative_not_EOF) is undefined behavior. Access via unsigned char * for maximum portability. A char may be negative.

// const char *p = s;
// ... 
// while( isspace( *p ) ) { 

const unsigned char *p = (const unsigned char *) s;
... 
while( isspace( *p ) ) { 

Do not use errno = ERANGE for a non-range issue

if( !isdigit( *p ) ) { 
  errno = ERANGE; // Do not do this.
  // Re-think error indication strategy.

Unneeded cast

// val = (uint128)( 10 * val ) + (uint128)( *p - '0' ); 
val = 10 * val + (uint128)( *p - '0' ); 

Use boolean assignments

neg is used as a boolean flag. Use boolean constants.

// bool neg = 0; 
bool neg = false; 

Detect buffer access

I'd expect u128toasc() to return with a failure indication if the supplied buffer is too small.

I recommend to use an internal buffer of sufficient size for all int128 and then instead of memmove( str, str + j, len - j ), check size requirements (maybe test len > j) and then memcpy(str, local_buffer + j, len - j).

Use 10 for base 10 conversions

// while( x < -9 ) {
while( x <= -10 ) {

Minor: simplify

// memmove( str, str + j, len - j );
// return( str ); 

return memmove( str, str + j, len - j );

Way too many comments

added 75 characters in body
Source Link
chux
  • 36.4k
  • 2
  • 43
  • 97

Don't bother defining U128_MIN

It is zero.

// #define U128_MIN (uint128)0
// #define U128_MAX (uint128)( ~U128_MIN )   
#define U128_MAX ((uint128)-1)

Style: Tabs vs. spaces

Using spaces is potential more profitable.

Undefined behavior

is...(some_negative_not_EOF) is undefined behavior. Access via unsigned char * for maximum portability. A char may be negative.

// const char *p = s;
// ... 
// while( isspace( *p ) ) { 

const unsigned char *p = (const unsigned char *) s;
... 
while( isspace( *p ) ) { 

Do not use errno = ERANGE for a non-range issue

if( !isdigit( *p ) ) { 
  errno = ERANGE; // Do not do this.
  // Re-think error indication strategy.

Unneeded cast

// val = (uint128)( 10 * val ) + (uint128)( *p - '0' ); 
val = 10 * val + (uint128)( *p - '0' ); 

Use boolean assignments

neg is used as a boolean flag. Use boolean constants.

// bool neg = 0; 
bool neg = false; 

Detect buffer access

I'd expect u128toasc() to return with a failure indication if the supplied buffer is too small.

I recommend to use an internal buffer of sufficient size for all int128 and then instead of memmmovememmove( str, str + j, len - j ), check size requirements (maybe test len > j) and then memcpy(str, local_buffer + j, len - j).

Use 10 for base 10 conversions

// while( x < -9 ) {
while( x <= -10 ) {

Minor: simplify

// memmove( str, str + j, len - j );
// return( str ); 

return memmove( str, str + j, len - j );

Don't bother defining U128_MIN

It is zero.

// #define U128_MIN (uint128)0
// #define U128_MAX (uint128)( ~U128_MIN )   
#define U128_MAX ((uint128)-1)

Style: Tabs vs. spaces

Using spaces is potential more profitable.

Undefined behavior

is...(some_negative_not_EOF) is undefined behavior. Access via unsigned char * for maximum portability. A char may be negative.

// const char *p = s;
// ... 
// while( isspace( *p ) ) { 

const unsigned char *p = (const unsigned char *) s;
... 
while( isspace( *p ) ) { 

Do not use errno = ERANGE for a non-range issue

if( !isdigit( *p ) ) { 
  errno = ERANGE; // Do not do this.
  // Re-think error indication strategy.

Unneeded cast

// val = (uint128)( 10 * val ) + (uint128)( *p - '0' ); 
val = 10 * val + (uint128)( *p - '0' ); 

Use boolean assignments

neg is used as a boolean flag. Use boolean constants.

// bool neg = 0; 
bool neg = false; 

Detect buffer access

I'd expect u128toasc() to return with a failure indication if the supplied buffer is too small.

I recommend to use an internal buffer of sufficient size for all int128 and then instead of memmmove(), check size requirements and then memcpy().

Use 10 for base 10 conversions

// while( x < -9 ) {
while( x <= -10 ) {

Minor: simplify

// memmove( str, str + j, len - j );
// return( str ); 

return memmove( str, str + j, len - j );

Don't bother defining U128_MIN

It is zero.

// #define U128_MIN (uint128)0
// #define U128_MAX (uint128)( ~U128_MIN )   
#define U128_MAX ((uint128)-1)

Style: Tabs vs. spaces

Using spaces is potential more profitable.

Undefined behavior

is...(some_negative_not_EOF) is undefined behavior. Access via unsigned char * for maximum portability. A char may be negative.

// const char *p = s;
// ... 
// while( isspace( *p ) ) { 

const unsigned char *p = (const unsigned char *) s;
... 
while( isspace( *p ) ) { 

Do not use errno = ERANGE for a non-range issue

if( !isdigit( *p ) ) { 
  errno = ERANGE; // Do not do this.
  // Re-think error indication strategy.

Unneeded cast

// val = (uint128)( 10 * val ) + (uint128)( *p - '0' ); 
val = 10 * val + (uint128)( *p - '0' ); 

Use boolean assignments

neg is used as a boolean flag. Use boolean constants.

// bool neg = 0; 
bool neg = false; 

Detect buffer access

I'd expect u128toasc() to return with a failure indication if the supplied buffer is too small.

I recommend to use an internal buffer of sufficient size for all int128 and then instead of memmove( str, str + j, len - j ), check size requirements (maybe test len > j) and then memcpy(str, local_buffer + j, len - j).

Use 10 for base 10 conversions

// while( x < -9 ) {
while( x <= -10 ) {

Minor: simplify

// memmove( str, str + j, len - j );
// return( str ); 

return memmove( str, str + j, len - j );
Source Link
chux
  • 36.4k
  • 2
  • 43
  • 97
Loading