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

sys/base64: api change (const + void*) #10053

Merged
merged 2 commits into from
Oct 23, 2018

Conversation

pwillem
Copy link

@pwillem pwillem commented Sep 26, 2018

Contribution description

This merge request changes the api of the base64 functions:

  • add const to input arguments
  • use void * for the raw (byte) data (input for base64_encode(), output for base64_decode())

Testing procedure

make BOARD=native -C tests/unittests tests-base64 test still compiles and passes

Issues/PRs references

None

@pwillem pwillem changed the title Feat/base64 const sys/base64 api change (const + void*) Sep 26, 2018
@pwillem pwillem changed the title sys/base64 api change (const + void*) sys/base64: api change (const + void*) Sep 26, 2018
@miri64 miri64 requested a review from smlng September 26, 2018 18:10
@miri64 miri64 added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 26, 2018
@smlng
Copy link
Member

smlng commented Sep 26, 2018

@pwillem I agree we[with] making the input const and I tend to agree to use void * to avoid (😉 ) casting when passing around buffers, see #5497 too. @kaspar030 what's your assessment on this here?

unsigned char *base64_out, size_t *base64_out_size)
{
const unsigned char *in = data_in;
Copy link
Contributor

Choose a reason for hiding this comment

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

is this variable necesary?

Copy link
Member

Choose a reason for hiding this comment

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

mhm, right I mean thats the point of using void right, to not hassle with casts.

Copy link
Author

Choose a reason for hiding this comment

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

if I try to use the void. I get the following errors:

RIOT/sys/base64/base64.c: In function ‘base64_encode’:
RIOT/sys/base64/base64.c:89:28: error: pointer of type ‘void *’ used in arithmetic [-Werror=pointer-arith]
         tmpval = *(data_in + i);
                            ^
RIOT/sys/base64/base64.c:89:18: error: dereferencing ‘void’ pointer [-Werror]
         tmpval = *(data_in + i);
                  ^~~~~~~~~~~~~~
RIOT/sys/base64/base64.c:89:16: error: void value not ignored as it ought to be
         tmpval = *(data_in + i);
                ^

Copy link
Contributor

Choose a reason for hiding this comment

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

That's right, void* cannot be used in pointer arithmetic.

@kaspar030
Copy link
Contributor

@kaspar030 what's your assessment on this here?

+1!

Copy link
Contributor

@jcarrano jcarrano left a comment

Choose a reason for hiding this comment

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

Looks good, and the test passes.

@jcarrano jcarrano merged commit 12729d6 into RIOT-OS:master Oct 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants