XRPL-Labs / xrpld-hooks

ISC License
94 stars 28 forks source link

Issue when using var names already defined in macros #58

Closed f1f47a23 closed 1 year ago

f1f47a23 commented 2 years ago

Issue Description

while I was studying and modifying the notary.c code, I noticed that variable "buf" had a wrong value. It seems that the problem is related to the "buf" variable name. To get around the problem I had to rename the variable to "buf2"

Steps to Reproduce

Below the changes to notary.c code to reproduce this issue

1) rename all "buf" statements to "buf2"

2) then on line 129 duplicate the buffer

uint8_t buf[4];
uint8_t buf2[4];

3) finally on line 290 modify as below

// the value we record against the signer is his/her signer weight at the time the endorsement or proposal happened

UINT16_TO_BUF(buf, signer_weight);
UINT16_TO_BUF(buf2, signer_weight);
trace("buf1", 4, buf, 2, 1);   // size is 2 + 2-not-used
trace("buf2", 4, buf2, 2, 1);  // size is 2 + 2-not-used

if (state_set(buf2, 2, SBUF(invoice_id)) != 2)
    rollback(SBUF("Notary: Could not write signature to hook state."), 7);

Expected Result

trace("buf1",.....) should log: "0002" trace("buf2",.....) logs: "0002"

Actual Result

trace("buf1",.....) logs: "0000" (WRONG!) trace("buf2",.....) logs: "0002"

Environment

online hooks builder https://hooks-builder.xrpl.org

UPDATE

Supporting Files

added fiile "notary-ISSUE-58-Weird-issue(buf-name).zip" including "notary.c.wasm" I did one more test before attaching file

notary-ISSUE-58-Weird-issue(buf-name).zip

f1f47a23 commented 2 years ago

.... and this is why .... beware of internal variables of the macros

define UINT16_TO_BUF(buf_raw, i)\

{\ unsigned char buf = (unsigned char)buf_raw;\ buf[0] = (((uint64_t)i) >> 8) & 0xFFUL;\ buf[1] = (((uint64_t)i) >> 0) & 0xFFUL;\ }

WietseWind commented 2 years ago

Nice find @f1f47a23! Thank you! Would make sense to change var names in macros to something no same person would ever use (?) @RichardAH

f1f47a23 commented 2 years ago

Hi. for example

define UINT16_TO_BUF(buf_raw, i)

{ unsigned char buf = (unsigned char)bufraw; buf _ [0] = (((uint64t)i) >> 8) & 0xFFUL; buf _ [1] = (((uint64_t)i) >> 0) & 0xFFUL; }

By the way, macros are a bit painful for me. I already asked if there is a way to enable inline functions, to be used in place of macros. I saw that there is also an "always inline" c compiler option.

RichardAH commented 1 year ago

All macros / compile-time helpers are being moved out of the main repo. Please re-open here if still relevant: https://github.com/XRPLF/hook-macros