Skip to main content
replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

Some additional thoughts after @Mat@Mat fine answer.

  1. When implementing an algorithm that counts on a particular size of integer, better to use a size that is guaranteed to work, or consider fixed size types. unsigned may only be 16-bit. Insufficient for the task.

     #include <stdint.h>
    
     // unsigned finalfinal;
     // Options in order of (my) preference
     uint32_t      finalfinal;
     unsigned long finalfinal;
     uint_fast32_t finalfinal;
    
  2. For maximum portability, consider that 16-bit machines are very popular in the embedded world in 2016, so do not assume unsigned/int math is at least 32-bit, code to C spec which says an unsigned may only be 16-bit. A 16-bit shift of text[x], which is promoted to int, may not work.

     // finalfinal = (text[x] << 16) | (text[x+1] << 8) | text[x+2];
     finalfinal = (text[x]*1UL << 16) | (text[x+1]*1UL << 8) | text[x+2];
     //  or 
     finalfinal = text[x];
     finalfinal <<= 8; 
     finalfinal |= text[x+1];
     finalfinal <<= 8; 
     finalfinal |= text[x+2];
    
     // final = text[text_len-1] << 16;
     final = text[text_len-1]*1UL << 16;
    
  3. Recommend similar thoughts for the function parameters, variables too.

    // char get_base64_digit(const unsigned int base64, int digit_number)
    int get_base64_digit(const uint32_t base64, unsigned digit_number)
    
  4. Extending the code to non-null character terminated sources. Current code works fine for test, yet all the following code should be wrapped in a function with the data pointer and size passed in.

     char* text = ...
     size_t text_len = strlen(text);
    
     // Something like this is the real function you need
     int status = NMK64_encode(text, text_len);
    
  5. funcs.c is not distinctive. How about NMK_Base64.c?

  6. Minor stuff follows

  7. Use matching type. strlen() return type size_t, not int.

     // int text_len  = strlen(text);
     // int remainder = text_len % 3;
     size_t text_len  = strlen(text);
     size_t remainder = text_len % 3;
    
  8. Make const

     // static char index_table[] = ...
     static const char index_table[] = ...
    
  9. Make static. Would not expect this helper function to have global usage. Expect NMK64_encode(text, text_len) to be the global interface.

     // get_base64_digit(
     static int get_base64_digit( ...
    

Some additional thoughts after @Mat fine answer.

  1. When implementing an algorithm that counts on a particular size of integer, better to use a size that is guaranteed to work, or consider fixed size types. unsigned may only be 16-bit. Insufficient for the task.

     #include <stdint.h>
    
     // unsigned finalfinal;
     // Options in order of (my) preference
     uint32_t      finalfinal;
     unsigned long finalfinal;
     uint_fast32_t finalfinal;
    
  2. For maximum portability, consider that 16-bit machines are very popular in the embedded world in 2016, so do not assume unsigned/int math is at least 32-bit, code to C spec which says an unsigned may only be 16-bit. A 16-bit shift of text[x], which is promoted to int, may not work.

     // finalfinal = (text[x] << 16) | (text[x+1] << 8) | text[x+2];
     finalfinal = (text[x]*1UL << 16) | (text[x+1]*1UL << 8) | text[x+2];
     //  or 
     finalfinal = text[x];
     finalfinal <<= 8; 
     finalfinal |= text[x+1];
     finalfinal <<= 8; 
     finalfinal |= text[x+2];
    
     // final = text[text_len-1] << 16;
     final = text[text_len-1]*1UL << 16;
    
  3. Recommend similar thoughts for the function parameters, variables too.

    // char get_base64_digit(const unsigned int base64, int digit_number)
    int get_base64_digit(const uint32_t base64, unsigned digit_number)
    
  4. Extending the code to non-null character terminated sources. Current code works fine for test, yet all the following code should be wrapped in a function with the data pointer and size passed in.

     char* text = ...
     size_t text_len = strlen(text);
    
     // Something like this is the real function you need
     int status = NMK64_encode(text, text_len);
    
  5. funcs.c is not distinctive. How about NMK_Base64.c?

  6. Minor stuff follows

  7. Use matching type. strlen() return type size_t, not int.

     // int text_len  = strlen(text);
     // int remainder = text_len % 3;
     size_t text_len  = strlen(text);
     size_t remainder = text_len % 3;
    
  8. Make const

     // static char index_table[] = ...
     static const char index_table[] = ...
    
  9. Make static. Would not expect this helper function to have global usage. Expect NMK64_encode(text, text_len) to be the global interface.

     // get_base64_digit(
     static int get_base64_digit( ...
    

Some additional thoughts after @Mat fine answer.

  1. When implementing an algorithm that counts on a particular size of integer, better to use a size that is guaranteed to work, or consider fixed size types. unsigned may only be 16-bit. Insufficient for the task.

     #include <stdint.h>
    
     // unsigned finalfinal;
     // Options in order of (my) preference
     uint32_t      finalfinal;
     unsigned long finalfinal;
     uint_fast32_t finalfinal;
    
  2. For maximum portability, consider that 16-bit machines are very popular in the embedded world in 2016, so do not assume unsigned/int math is at least 32-bit, code to C spec which says an unsigned may only be 16-bit. A 16-bit shift of text[x], which is promoted to int, may not work.

     // finalfinal = (text[x] << 16) | (text[x+1] << 8) | text[x+2];
     finalfinal = (text[x]*1UL << 16) | (text[x+1]*1UL << 8) | text[x+2];
     //  or 
     finalfinal = text[x];
     finalfinal <<= 8; 
     finalfinal |= text[x+1];
     finalfinal <<= 8; 
     finalfinal |= text[x+2];
    
     // final = text[text_len-1] << 16;
     final = text[text_len-1]*1UL << 16;
    
  3. Recommend similar thoughts for the function parameters, variables too.

    // char get_base64_digit(const unsigned int base64, int digit_number)
    int get_base64_digit(const uint32_t base64, unsigned digit_number)
    
  4. Extending the code to non-null character terminated sources. Current code works fine for test, yet all the following code should be wrapped in a function with the data pointer and size passed in.

     char* text = ...
     size_t text_len = strlen(text);
    
     // Something like this is the real function you need
     int status = NMK64_encode(text, text_len);
    
  5. funcs.c is not distinctive. How about NMK_Base64.c?

  6. Minor stuff follows

  7. Use matching type. strlen() return type size_t, not int.

     // int text_len  = strlen(text);
     // int remainder = text_len % 3;
     size_t text_len  = strlen(text);
     size_t remainder = text_len % 3;
    
  8. Make const

     // static char index_table[] = ...
     static const char index_table[] = ...
    
  9. Make static. Would not expect this helper function to have global usage. Expect NMK64_encode(text, text_len) to be the global interface.

     // get_base64_digit(
     static int get_base64_digit( ...
    
deleted 7 characters in body
Source Link
chux
  • 33.7k
  • 2
  • 38
  • 92

Some additional thoughts after @Mat fine answer.

  1. When implementing an algorithm that counts on a particular size of integer, better to use a size that is guaranteed to work, or consider fixed size types. unsigned may only be 16-bit. Insufficient for the task.

     #include <stdint.h>
    
     // unsigned finalfinal;
     // Options in order of (my) preference
     uint32_t      finalfinal;
     unsigned long finalfinal;
     uint_fast32_t finalfinal;
    
  2. For maximum portability, consider that 16-bit machines are very popular in the embedded world in 2016, so do not assume unsigned/int math is at least 32-bit, code to C spec which says an unsigned may only be 16-bit. A 16-bit shift of text[x], which is promoted to int, may not work.

     // finalfinal = (text[x] << 16) | (text[x+1] << 8) | text[x+2];
     finalfinal = (text[x]*1UL << 16) | (text[x+1]*1UL << 8) | text[x+2];
     //  or 
     finalfinal = text[x];
     finalfinal <<= 8; 
     finalfinal |= text[x+1];
     finalfinal <<= 8; 
     finalfinal |= text[x+2];
    
     // final = text[text_len-1] << 16;
     final = text[text_len-1]*1UL << 16;
    
  3. Recommend similar thoughts for the function parameters, variables too.

    // char get_base64_digit(const unsigned int base64, int digit_number)
    int get_base64_digit(const uint32_t base64, unsigned digit_number)
    
  4. Extending the code to non-nulllnull character terminated sources. Current code works fine for test, yet all the following code should be wrapped in a function with the data pointer and size passed in.

     char* text = ...
     size_t text_len = strlen(text);
    
     // Something like this is the real function you need
     int status = NMK64_encode(text, text_len);
    
  5. funcs.c is not distinctive. How about NMK_Base64.c?

  6. Minor stuff follows

  7. Use matching type. strlen() return type size_t, not int.

     // int text_len  = strlen(text);
     // int remainder = text_len % 3;
     size_t text_len  = strlen(text);
     size_t remainder = text_len % 3;
    
  8. Make const

     // static char index_table[] = ...
     static const char index_table[] = ...
    
  9. Make static. Would not expect this helper function to have global usage. Expect NMK64_encode(text, text_len) to be the global interface.

     // get_base64_digit(
     static int get_base64_digit( ...
    

Some additional thoughts after @Mat fine answer.

  1. When implementing an algorithm that counts on a particular size of integer, better to use a size that is guaranteed to work, or consider fixed size types. unsigned may only be 16-bit. Insufficient for the task.

     #include <stdint.h>
    
     // unsigned finalfinal;
     // Options in order of (my) preference
     uint32_t      finalfinal;
     unsigned long finalfinal;
     uint_fast32_t finalfinal;
    
  2. For maximum portability, consider that 16-bit machines are very popular in the embedded world in 2016, so do not assume unsigned/int math is at least 32-bit, code to C spec which says an unsigned may only be 16-bit. A 16-bit shift of text[x], which is promoted to int, may not work.

     // finalfinal = (text[x] << 16) | (text[x+1] << 8) | text[x+2];
     finalfinal = (text[x]*1UL << 16) | (text[x+1]*1UL << 8) | text[x+2];
     //  or 
     finalfinal = text[x];
     finalfinal <<= 8; 
     finalfinal |= text[x+1];
     finalfinal <<= 8; 
     finalfinal |= text[x+2];
    
     // final = text[text_len-1] << 16;
     final = text[text_len-1]*1UL << 16;
    
  3. Recommend similar thoughts for the function parameters, variables too.

    // char get_base64_digit(const unsigned int base64, int digit_number)
    int get_base64_digit(const uint32_t base64, unsigned digit_number)
    
  4. Extending the code to non-nulll character terminated sources. Current code works fine for test, yet all the following code should be wrapped in a function with the data pointer and size passed in.

     char* text = ...
     size_t text_len = strlen(text);
    
     // Something like this is the real function you need
     int status = NMK64_encode(text, text_len);
    
  5. funcs.c is not distinctive. How about NMK_Base64.c?

  6. Minor stuff follows

  7. Use matching type. strlen() return type size_t, not int.

     // int text_len  = strlen(text);
     // int remainder = text_len % 3;
     size_t text_len  = strlen(text);
     size_t remainder = text_len % 3;
    
  8. Make const

     // static char index_table[] = ...
     static const char index_table[] = ...
    
  9. Make static. Would not expect this helper function to have global usage. Expect NMK64_encode(text, text_len) to be the global interface.

     // get_base64_digit(
     static int get_base64_digit( ...
    

Some additional thoughts after @Mat fine answer.

  1. When implementing an algorithm that counts on a particular size of integer, better to use a size that is guaranteed to work, or consider fixed size types. unsigned may only be 16-bit. Insufficient for the task.

     #include <stdint.h>
    
     // unsigned finalfinal;
     // Options in order of (my) preference
     uint32_t      finalfinal;
     unsigned long finalfinal;
     uint_fast32_t finalfinal;
    
  2. For maximum portability, consider that 16-bit machines are very popular in the embedded world in 2016, so do not assume unsigned/int math is at least 32-bit, code to C spec which says an unsigned may only be 16-bit. A 16-bit shift of text[x], which is promoted to int, may not work.

     // finalfinal = (text[x] << 16) | (text[x+1] << 8) | text[x+2];
     finalfinal = (text[x]*1UL << 16) | (text[x+1]*1UL << 8) | text[x+2];
     //  or 
     finalfinal = text[x];
     finalfinal <<= 8; 
     finalfinal |= text[x+1];
     finalfinal <<= 8; 
     finalfinal |= text[x+2];
    
     // final = text[text_len-1] << 16;
     final = text[text_len-1]*1UL << 16;
    
  3. Recommend similar thoughts for the function parameters, variables too.

    // char get_base64_digit(const unsigned int base64, int digit_number)
    int get_base64_digit(const uint32_t base64, unsigned digit_number)
    
  4. Extending the code to non-null character terminated sources. Current code works fine for test, yet all the following code should be wrapped in a function with the data pointer and size passed in.

     char* text = ...
     size_t text_len = strlen(text);
    
     // Something like this is the real function you need
     int status = NMK64_encode(text, text_len);
    
  5. funcs.c is not distinctive. How about NMK_Base64.c?

  6. Minor stuff follows

  7. Use matching type. strlen() return type size_t, not int.

     // int text_len  = strlen(text);
     // int remainder = text_len % 3;
     size_t text_len  = strlen(text);
     size_t remainder = text_len % 3;
    
  8. Make const

     // static char index_table[] = ...
     static const char index_table[] = ...
    
  9. Make static. Would not expect this helper function to have global usage. Expect NMK64_encode(text, text_len) to be the global interface.

     // get_base64_digit(
     static int get_base64_digit( ...
    
Source Link
chux
  • 33.7k
  • 2
  • 38
  • 92

Some additional thoughts after @Mat fine answer.

  1. When implementing an algorithm that counts on a particular size of integer, better to use a size that is guaranteed to work, or consider fixed size types. unsigned may only be 16-bit. Insufficient for the task.

     #include <stdint.h>
    
     // unsigned finalfinal;
     // Options in order of (my) preference
     uint32_t      finalfinal;
     unsigned long finalfinal;
     uint_fast32_t finalfinal;
    
  2. For maximum portability, consider that 16-bit machines are very popular in the embedded world in 2016, so do not assume unsigned/int math is at least 32-bit, code to C spec which says an unsigned may only be 16-bit. A 16-bit shift of text[x], which is promoted to int, may not work.

     // finalfinal = (text[x] << 16) | (text[x+1] << 8) | text[x+2];
     finalfinal = (text[x]*1UL << 16) | (text[x+1]*1UL << 8) | text[x+2];
     //  or 
     finalfinal = text[x];
     finalfinal <<= 8; 
     finalfinal |= text[x+1];
     finalfinal <<= 8; 
     finalfinal |= text[x+2];
    
     // final = text[text_len-1] << 16;
     final = text[text_len-1]*1UL << 16;
    
  3. Recommend similar thoughts for the function parameters, variables too.

    // char get_base64_digit(const unsigned int base64, int digit_number)
    int get_base64_digit(const uint32_t base64, unsigned digit_number)
    
  4. Extending the code to non-nulll character terminated sources. Current code works fine for test, yet all the following code should be wrapped in a function with the data pointer and size passed in.

     char* text = ...
     size_t text_len = strlen(text);
    
     // Something like this is the real function you need
     int status = NMK64_encode(text, text_len);
    
  5. funcs.c is not distinctive. How about NMK_Base64.c?

  6. Minor stuff follows

  7. Use matching type. strlen() return type size_t, not int.

     // int text_len  = strlen(text);
     // int remainder = text_len % 3;
     size_t text_len  = strlen(text);
     size_t remainder = text_len % 3;
    
  8. Make const

     // static char index_table[] = ...
     static const char index_table[] = ...
    
  9. Make static. Would not expect this helper function to have global usage. Expect NMK64_encode(text, text_len) to be the global interface.

     // get_base64_digit(
     static int get_base64_digit( ...