Skip to content

Conversation

@robertsipka
Copy link
Contributor

JerryScript-DCO-1.0-Signed-off-by: Robert Sipka rsipka.uszeged@partner.samsung.com

@LaszloLango LaszloLango added the bug Undesired behaviour label Mar 7, 2016
@LaszloLango
Copy link
Contributor

LGTM

int jerry_port_putchar (int c) /**< character to put */
{
return putchar (c);
return putchar ((unsigned char) c);
Copy link
Member

Choose a reason for hiding this comment

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

Why this is a problem?

http://www.cplusplus.com/reference/cstdio/putchar/

putchar should accept int argument.

@robertsipka
Copy link
Contributor Author

@zherczeg : A conversion error (conversion to 'unsigned char' from 'int' may alter its value) occurs when I tried to build to mbedk64f target with enabled USE_COMPILER_DEFAULT_LIBC.
It's the default value of this option (hard-coded in the Makefile.mbedk64f), and the same happens when I compile to another target with the following options:
USE_COMPILER_DEFAULT_LIBC=YES make release.mcu_stm32f3-cp
As I assume this is a compiler-specific error, and I dared to casting int to unsigned char because the value is internally converted to an unsigned char anyway.

@zherczeg
Copy link
Member

zherczeg commented Mar 8, 2016

Btw could we make jerry_port_putchar to accept unsigned char? Is it frequently used?

@robertsipka
Copy link
Contributor Author

No, it's very rarely used, but we couldn't get away with the cast, for example the following line :
jerry_port_putchar (ecma_char)
will be reported the same error, because the type of the variable is an ecma_char_t which is a typedef to uint16_t.
(In case of this kind of build configuration)

@zherczeg
Copy link
Member

zherczeg commented Mar 9, 2016

LGTM

JerryScript-DCO-1.0-Signed-off-by: Robert Sipka rsipka.uszeged@partner.samsung.com
@robertsipka
Copy link
Contributor Author

Rebased with master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Undesired behaviour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants