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

plugins/bcli: strip trailing whitespace appropriately #4502

Merged
merged 1 commit into from
May 3, 2021

Conversation

nalinbhardwaj
Copy link
Contributor

Fixes #4001

@nalinbhardwaj nalinbhardwaj marked this pull request as ready for review May 1, 2021 19:02
plugins/bcli.c Outdated
bcli->output[bcli->output_bytes-1] = 0x00;
/* Strip trailing whitespace and steal onto the stash. */
size_t striplen = bcli->output_bytes;
while (isspace(bcli->output[striplen-1]))
Copy link
Contributor

Choose a reason for hiding this comment

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

We have a cisspace helper which is sightly better here, BTW.

Can we extract this into a "static void strip_trailing_whitespace(char *str, size_t len)"
which also makes sure it doesn't underflow if it's all whitespace?

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@nalinbhardwaj nalinbhardwaj force-pushed the whitespace-bcli branch 2 times, most recently from 7f2d754 to 7ca3a37 Compare May 2, 2021 15:15
Changelog-Fixed: Handle windows-style newlines and other trailing whitespaces correctly in bitcoin-cli interface
Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Ack bd59929

@rustyrussell rustyrussell merged commit ae06bb9 into ElementsProject:master May 3, 2021
@nalinbhardwaj nalinbhardwaj deleted the whitespace-bcli branch May 3, 2021 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bcli: Bitcoin backend assumes Linux-style newlines
2 participants