variable size_t j not initlized

[alex-lt-kong]

In line 149 of https://github.com/bitcoin/libbase58/blob/b1dd03fa8d1be4be076bb6152325c6b5cf64f678/base58.c#L149, it appears to me that j is not initialized and directly used in line 159 to initialize high: https://github.com/bitcoin/libbase58/blob/b1dd03fa8d1be4be076bb6152325c6b5cf64f678/base58.c#L159.

Should we initialize j with 0?

For whatever reason, the code works at its current shape, but if I change carry to a local variable, i,e, from:

for (carry = bin[i], j = size - 1; (j > high) || carry; --j)
{
	carry += 256 * buf[j];
	buf[j] = carry % 58;
	carry /= 58;
	if (!j) {
		// Otherwise j wraps to maxint which is > high
		break;
	}
}

to

for (size_t carry = bin[i], j = size - 1; (j > high) || carry; --j)
{
	carry += 256 * buf[j];
	buf[j] = carry % 58;
	carry /= 58;
	if (!j) {
		// Otherwise j wraps to maxint which is > high
		break;
	}
}

the code gives me wrong base58 encoding result. If I initialize j with 0, the result will be correct again.

[katesalazar]

Kinda agree, will have a look. On first sight, tending to concept ACK

[lukellmann]

In line 149 of

https://github.com/bitcoin/libbase58/blob/b1dd03fa8d1be4be076bb6152325c6b5cf64f678/base58.c#L149 , it appears to me that j is not initialized and directly used in line 159 to initialize high:

https://github.com/bitcoin/libbase58/blob/b1dd03fa8d1be4be076bb6152325c6b5cf64f678/base58.c#L159 .

The third expression in a for statement (++i, high = j in this case) is evaluated after each execution of the loop body. The loop body always assigns to j in the first expression (carry = bin[i], j = size - 1) of the inner for loop:

for (carry = bin[i], j = size - 1; (j > high) || carry; --j)
{
	// ...
}

So j will be initialized after the first execution of the loop body before it is assigned to high. If the loop body is never executed, the assignment to high will not take place either.

---

For whatever reason, the code works at its current shape, but if I change carry to a local variable, i,e, from:

for (carry = bin[i], j = size - 1; (j > high) || carry; --j)
{
	carry += 256 * buf[j];
	buf[j] = carry % 58;
	carry /= 58;
	if (!j) {
		// Otherwise j wraps to maxint which is > high
		break;
	}
}

to

for (size_t carry = bin[i], j = size - 1; (j > high) || carry; --j)
{
	carry += 256 * buf[j];
	buf[j] = carry % 58;
	carry /= 58;
	if (!j) {
		// Otherwise j wraps to maxint which is > high
		break;
	}
}

the code gives me wrong base58 encoding result. If I initialize j with 0, the result will be correct again.

The reason is that size_t carry = bin[i], j = size - 1 is a declaration and initialization of two variables of type size_t: carry and j. So this declaration of j will shadow the previous one. The outer j will now not be initialized and the assignment to high at the end of the outer loop will use the uninitialized outer j.