Skip to main content
added 61 characters in body
Source Link
Madagascar
  • 10.1k
  • 1
  • 16
  • 52

The functions that are not used outside of a translation unit should be defined as having internal linkage:

Everything except sha256() should be defined with the static keyword.

#if 0
uint32_t right_rotate(uint32_t x, size_t k) {
    return ((x >> k) | (x << (32 - k)));
}
#else
static uint32_t right_rotate(uint32_t x, size_t k) {
    return ((x >> k) | (x << (32 - k)));
}
#endif

// And so on...

Error messages go to stderr:

I suggest:

#if 0
if (ptr == NULL) {
    printf("Unable to allocate enough memory!!\n");
    exit(EXIT_FAILURE);
}
#else

#include <errno.h>

errno = 0;
...
if (ptr == NULL) {
    if (errno) {
        perror("malloc()");
    } else {
        fputs("Unable to allocate enough memory!!\n", stderr);
    }
    exit(EXIT_FAILURE);
}
#endif

See: Why use stderr when printf works fine?

file_size() risks invoking undefined behavior:

From the C StandardC Standard: (footnote 234 on p. 267 of the linked Standard)

Setting the file position indicator to end-of-file, as with fseek(file, 0, SEEK_END),has undefined behavior for a binary stream (because of possible trailing null characters) or for anystream with state-dependent encoding that does not assuredly end in the initial shift state.

If the file was opened in binary mode, fseek(file_ptr, 0, SEEK_END); would invoke undefined behavior.

Don't include unnecessary header files:

sha256.h doesn't require anything but stdint.h and stddef.h.

#if 0
#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#else
#include <stdint.h>
#include <stddef.h>  /* This suffices for the definition of size_t. */
#endif

sha256.c should not depend on sha256.h. Use the headers you need per file, no more, no less.

Minor:

You don't need the extra parenthesis in the calls to memcpy(). You can safely drop them:

#if 0
memcpy((*buffer), message, message_length);
#else 
memcpy(*buffer, message, message_length);
#endif

The functions that are not used outside of a translation unit should be defined as having internal linkage:

Everything except sha256() should be defined with the static keyword.

#if 0
uint32_t right_rotate(uint32_t x, size_t k) {
    return ((x >> k) | (x << (32 - k)));
}
#else
static uint32_t right_rotate(uint32_t x, size_t k) {
    return ((x >> k) | (x << (32 - k)));
}
#endif

// And so on...

Error messages go to stderr:

I suggest:

#if 0
if (ptr == NULL) {
    printf("Unable to allocate enough memory!!\n");
    exit(EXIT_FAILURE);
}
#else

#include <errno.h>

errno = 0;
...
if (ptr == NULL) {
    if (errno) {
        perror("malloc()");
    } else {
        fputs("Unable to allocate enough memory!!\n", stderr);
    }
    exit(EXIT_FAILURE);
}
#endif

See: Why use stderr when printf works fine?

file_size() risks invoking undefined behavior:

From the C Standard: (footnote 234 on p. 267 of the linked Standard)

Setting the file position indicator to end-of-file, as with fseek(file, 0, SEEK_END),has undefined behavior for a binary stream (because of possible trailing null characters) or for anystream with state-dependent encoding that does not assuredly end in the initial shift state.

If the file was opened in binary mode, fseek(file_ptr, 0, SEEK_END); would invoke undefined behavior.

Don't include unnecessary header files:

sha256.h doesn't require anything but stdint.h and stddef.h.

#if 0
#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#else
#include <stdint.h>
#include <stddef.h>  /* This suffices for the definition of size_t. */
#endif

sha256.c should not depend on sha256.h. Use the headers you need per file, no more, no less.

Minor:

You don't need the extra parenthesis in the calls to memcpy(). You can safely drop them:

#if 0
memcpy((*buffer), message, message_length);
#else 
memcpy(*buffer, message, message_length);
#endif

The functions that are not used outside of a translation unit should be defined as having internal linkage:

Everything except sha256() should be defined with the static keyword.

#if 0
uint32_t right_rotate(uint32_t x, size_t k) {
    return ((x >> k) | (x << (32 - k)));
}
#else
static uint32_t right_rotate(uint32_t x, size_t k) {
    return ((x >> k) | (x << (32 - k)));
}
#endif

// And so on...

Error messages go to stderr:

I suggest:

#if 0
if (ptr == NULL) {
    printf("Unable to allocate enough memory!!\n");
    exit(EXIT_FAILURE);
}
#else

#include <errno.h>

errno = 0;
...
if (ptr == NULL) {
    if (errno) {
        perror("malloc()");
    } else {
        fputs("Unable to allocate enough memory!!\n", stderr);
    }
    exit(EXIT_FAILURE);
}
#endif

See: Why use stderr when printf works fine?

file_size() risks invoking undefined behavior:

From the C Standard: (footnote 234 on p. 267 of the linked Standard)

Setting the file position indicator to end-of-file, as with fseek(file, 0, SEEK_END),has undefined behavior for a binary stream (because of possible trailing null characters) or for anystream with state-dependent encoding that does not assuredly end in the initial shift state.

If the file was opened in binary mode, fseek(file_ptr, 0, SEEK_END); would invoke undefined behavior.

Don't include unnecessary header files:

sha256.h doesn't require anything but stdint.h and stddef.h.

#if 0
#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#else
#include <stdint.h>
#include <stddef.h>  /* This suffices for the definition of size_t. */
#endif

sha256.c should not depend on sha256.h. Use the headers you need per file, no more, no less.

Minor:

You don't need the extra parenthesis in the calls to memcpy(). You can safely drop them:

#if 0
memcpy((*buffer), message, message_length);
#else 
memcpy(*buffer, message, message_length);
#endif
Source Link
Madagascar
  • 10.1k
  • 1
  • 16
  • 52

The functions that are not used outside of a translation unit should be defined as having internal linkage:

Everything except sha256() should be defined with the static keyword.

#if 0
uint32_t right_rotate(uint32_t x, size_t k) {
    return ((x >> k) | (x << (32 - k)));
}
#else
static uint32_t right_rotate(uint32_t x, size_t k) {
    return ((x >> k) | (x << (32 - k)));
}
#endif

// And so on...

Error messages go to stderr:

I suggest:

#if 0
if (ptr == NULL) {
    printf("Unable to allocate enough memory!!\n");
    exit(EXIT_FAILURE);
}
#else

#include <errno.h>

errno = 0;
...
if (ptr == NULL) {
    if (errno) {
        perror("malloc()");
    } else {
        fputs("Unable to allocate enough memory!!\n", stderr);
    }
    exit(EXIT_FAILURE);
}
#endif

See: Why use stderr when printf works fine?

file_size() risks invoking undefined behavior:

From the C Standard: (footnote 234 on p. 267 of the linked Standard)

Setting the file position indicator to end-of-file, as with fseek(file, 0, SEEK_END),has undefined behavior for a binary stream (because of possible trailing null characters) or for anystream with state-dependent encoding that does not assuredly end in the initial shift state.

If the file was opened in binary mode, fseek(file_ptr, 0, SEEK_END); would invoke undefined behavior.

Don't include unnecessary header files:

sha256.h doesn't require anything but stdint.h and stddef.h.

#if 0
#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#else
#include <stdint.h>
#include <stddef.h>  /* This suffices for the definition of size_t. */
#endif

sha256.c should not depend on sha256.h. Use the headers you need per file, no more, no less.

Minor:

You don't need the extra parenthesis in the calls to memcpy(). You can safely drop them:

#if 0
memcpy((*buffer), message, message_length);
#else 
memcpy(*buffer, message, message_length);
#endif