Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Develop/holland01 #7

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 92 additions & 31 deletions base58.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,31 @@
* under the terms of the standard MIT license. See COPYING for more details.
*/

#ifndef WIN32
#include "libbase58.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this being moved?

Copy link
Author

@holland01 holland01 Jun 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stdint.h is in libbase58.h - see here.

stdint.h is needed in the header for msvc compilation.

I also didn't see any reason include 4 header files before the libbas58.h file, considering that most of those aren't necessary for the function declarations found in it.


#ifndef _WIN32
#include <arpa/inet.h>
#else
#include <winsock2.h>
#include <malloc.h>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There doesn't seem to be any malloc.h on at least Mingw cross-compilers.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll replace with an appropriate _MSC_VER check.


#include <stdio.h>

#ifdef _WIN64
typedef int64_t ssize_t;
#else
typedef int32_t ssize_t;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would prefer to just avoid ssize_t (use size_t and avoid circumstances that can lead to a negative value) rather than try to typedef it. Pretty sure this will break on Mingw.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm honestly wondering why ssize_t existed there in the first place, then?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Convenience; j can get to -1 with the current code. So we need to handle it differently to avoid that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved this issue to #8; please review

#endif
#endif

#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still need stdint.h here, as this file uses intN_t types.

Copy link
Author

@holland01 holland01 Jun 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be included in files where it is used directly.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, fair enough.

#include <string.h>
#include <stdlib.h>

#include <sys/types.h>

#include "libbase58.h"


bool (*b58_sha256_impl)(void *, const void *, size_t) = NULL;

Expand All @@ -37,39 +49,69 @@ typedef uint32_t b58_almostmaxint_t;
#define b58_almostmaxint_bits (sizeof(b58_almostmaxint_t) * 8)
static const b58_almostmaxint_t b58_almostmaxint_mask = ((((b58_maxint_t)1) << b58_almostmaxint_bits) - 1);

// MSVC 2017 doesn't support the GCC (and probably clang) extension
// for dynamic arrays in C
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable length arrays are a standard C99 feature, not an extension. The library is intentionally designed to not use malloc/calloc/free.

Copy link
Author

@holland01 holland01 Jun 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable length arrays are a standard C99 feature, not an extension. The library is intentionally designed to not use malloc/calloc/free.

That may be, but MSVC still doesn't support them. _alloca or the MSVC _malloca can be used if that's preferred - these perform stack allocation (though _malloca will allocate heap for sufficiently large sizes).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK, MSVC is a C++ compiler, not a C compiler.

I suppose it can't hurt to use _malloca or such, based on a failed configure check for standards compatibility...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few years back they added C99 support. It's still subpar in comparison to MinGW/GCC, but it's better than it used to be - enough to where people who like C end up using it over C++.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrote a simple alternative in #9; can you check if it works?

#ifdef _WIN32
#define b58_log_err(msg, ...) printf("[" __FUNCTION__ " - ERROR]: " msg, __VA_ARGS__)

#define b58_alloc_mem(type, name, count) \
type *name = NULL; \
do { \
name = calloc(count, sizeof(type)); \
if (!name) { \
b58_log_err("%s", "Could not allocate " #name); \
return false; \
} \
} while (0)

#define b58_free_mem(v) \
do { \
if ((v)) { \
free((v)); \
} \
} while (0)
#else
#define b58_alloc_mem(type, name, count) type name[count]
#define b58_free_mem(v)
#endif

bool b58tobin(void *bin, size_t *binszp, const char *b58, size_t b58sz)
{
size_t binsz = *binszp;
const unsigned char *b58u = (void*)b58;
unsigned char *binu = bin;
size_t outisz = (binsz + sizeof(b58_almostmaxint_t) - 1) / sizeof(b58_almostmaxint_t);
b58_almostmaxint_t outi[outisz];

// b58_almostmaxint_t outi[outisz];

b58_alloc_mem(b58_almostmaxint_t, outi, outisz);

b58_maxint_t t;
b58_almostmaxint_t c;
size_t i, j;
uint8_t bytesleft = binsz % sizeof(b58_almostmaxint_t);
b58_almostmaxint_t zeromask = bytesleft ? (b58_almostmaxint_mask << (bytesleft * 8)) : 0;
unsigned zerocount = 0;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't mess with the formatting.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies, I'll be sure to omit any of those in the future.

if (!b58sz)
b58sz = strlen(b58);

for (i = 0; i < outisz; ++i) {
outi[i] = 0;
}

// Leading zeros, just count
for (i = 0; i < b58sz && b58u[i] == '1'; ++i)
++zerocount;

for ( ; i < b58sz; ++i)
{
if (b58u[i] & 0x80)
// High-bit set on invalid digit
return false;
goto ret_false;
if (b58digits_map[b58u[i]] == -1)
// Invalid base58 digit
return false;
goto ret_false;
c = (unsigned)b58digits_map[b58u[i]];
for (j = outisz; j--; )
{
Expand All @@ -79,27 +121,27 @@ bool b58tobin(void *bin, size_t *binszp, const char *b58, size_t b58sz)
}
if (c)
// Output number too big (carry to the next int32)
return false;
goto ret_false;
if (outi[0] & zeromask)
// Output number too big (last int32 filled too far)
return false;
goto ret_false;
}

j = 0;
if (bytesleft) {
for (i = bytesleft; i > 0; --i) {
*(binu++) = (outi[0] >> (8 * (i - 1))) & 0xff;
}
++j;
}

for (; j < outisz; ++j)
{
for (i = sizeof(*outi); i > 0; --i) {
*(binu++) = (outi[j] >> (8 * (i - 1))) & 0xff;
}
}

// Count canonical base58 byte count
binu = bin;
for (i = 0; i < binsz; ++i)
Expand All @@ -109,8 +151,13 @@ bool b58tobin(void *bin, size_t *binszp, const char *b58, size_t b58sz)
--*binszp;
}
*binszp += zerocount;


b58_free_mem(outi);
return true;

ret_false:
b58_free_mem(outi);
return false;
}

static
Expand All @@ -131,13 +178,13 @@ int b58check(const void *bin, size_t binsz, const char *base58str, size_t b58sz)
return -2;
if (memcmp(&binc[binsz - 4], buf, 4))
return -1;

// Check number of zeros is correct AFTER verifying checksum (to avoid possibility of accessing base58str beyond the end)
for (i = 0; binc[i] == '\0' && base58str[i] == '1'; ++i)
{} // Just finding the end of zeros, nothing to do in loop
if (binc[i] == '\0' || base58str[i] == '1')
return -3;

return binc[0];
}

Expand All @@ -149,14 +196,16 @@ bool b58enc(char *b58, size_t *b58sz, const void *data, size_t binsz)
int carry;
ssize_t i, j, high, zcount = 0;
size_t size;

while (zcount < binsz && !bin[zcount])
++zcount;

size = (binsz - zcount) * 138 / 100 + 1;
uint8_t buf[size];
//uint8_t buf[size];

b58_alloc_mem(uint8_t, buf, size);
memset(buf, 0, size);

for (i = zcount, high = size - 1; i < binsz; ++i, high = j)
{
for (carry = bin[i], j = size - 1; (j > high) || carry; --j)
Expand All @@ -166,37 +215,49 @@ bool b58enc(char *b58, size_t *b58sz, const void *data, size_t binsz)
carry /= 58;
}
}

for (j = 0; j < size && !buf[j]; ++j);

if (*b58sz <= zcount + size - j)
{
*b58sz = zcount + size - j + 1;
return false;

goto error;
}

if (zcount)
memset(b58, '1', zcount);
for (i = zcount; j < size; ++i, ++j)
b58[i] = b58digits_ordered[buf[j]];
b58[i] = '\0';
*b58sz = i + 1;

return true;

error:
b58_free_mem(buf);
return false;
}

bool b58check_enc(char *b58c, size_t *b58c_sz, uint8_t ver, const void *data, size_t datasz)
{
uint8_t buf[1 + datasz + 0x20];
//uint8_t buf[1 + datasz + 0x20];

b58_alloc_mem(uint8_t, buf, 1 + datasz + 0x20);

uint8_t *hash = &buf[1 + datasz];

buf[0] = ver;
memcpy(&buf[1], data, datasz);
if (!my_dblsha256(hash, buf, datasz + 1))
{
*b58c_sz = 0;

b58_free_mem(buf);
return false;
}

return b58enc(b58c, b58c_sz, buf, 1 + datasz + 4);

bool rc = b58enc(b58c, b58c_sz, buf, 1 + datasz + 4);
b58_free_mem(buf);
return rc;
}
1 change: 1 addition & 0 deletions libbase58.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>

#ifdef __cplusplus
extern "C" {
Expand Down