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

Set -funsigned-char flag when exporting to GCC #1525

Closed
aamcampbell opened this issue Feb 3, 2016 · 8 comments
Closed

Set -funsigned-char flag when exporting to GCC #1525

aamcampbell opened this issue Feb 3, 2016 · 8 comments

Comments

@aamcampbell
Copy link

Here's a minor feature request for exporting to GCC and GCC-based environments (such as KDS and Atmel Studio): add the compiler flag-funsigned-char in exported projects by default.
)
The mbed handbook lists the char type as being unsigned (here), but the C standard does not specify, and in GCC chars are signed by default (see http://stackoverflow.com/a/2054959/1858923). This leads to errors when exporting projects that assume that chars are unsigned, such as Neil Thiessen's SDFileSystem. If we automatically set -funsigned-char in exported projects, we reduce the inconsistencies between the mbed online compiler and other target compilers for exported projects.

@sg-
Copy link
Contributor

sg- commented Feb 11, 2016

👍

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 11, 2016

@aamcampbell Please send PR to workspace_tools/toolchain scripts, with a fix

@adamgreen
Copy link
Contributor

Pros: Makes GCC builds the same as the online compiler.
Cons: Promotes writing code which isn't really portable. The correct solution would be for the developer to be explicit with signed/unsigned designation for their char variables (most easily by using the stdint.h types).

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 11, 2016

Adam, it was stated that mbed library or anything else, hosted on mbed.org and used in the online compiler , have the assumption that char is unsigned. Therefore users wrote the libraries with this assumption, which of course means it's not portable :-/

I am in favor of consistency, and we should state this in the document (where the info is that char is unsigned in the online compiler, that this might be valid for the online compiler, but not of the world outside :) ).

@adamgreen
Copy link
Contributor

Go for it. I just tested with GCC-ARM toolchain and it appears to be unsigned there by default too.

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 11, 2016

@aamcampbell I would like to know more details where did you have this problem. The one you referenced is for gcc but not specifying the platform.. GCC ARM for instance as Adam pointed out above defaults char to be unsigned. Aren't Atmel and KSD using GCC ARM?

If you can share an example which I can export and get those warnings, would be helpful.

@aamcampbell
Copy link
Author

@0xc0170 Sure. Neil Thiessen's SDFileSystem is the project where I ran into this problem, exported to Kinetis Design Studio, running on a FRDM-K64F. KDS does use arm-none-eabi-gcc, but clearly defaults to signed chars. (See also: https://community.freescale.com/thread/335786)

Steps to reproduce:

DigitalIn button(PTC6, PullUp);
SDFileSystem sd(PTE3, PTE1, PTE2, PTE4, "sd", PTE6, SDFileSystem::SWITCH_NEG_NO, 25000000);
  • Export to Kinetis Design Studio, for platform FRDM-K64F
  • Set a breakpoint on line 443, return (resp > 0x00);
  • Compile and run on the target board, with an SD card inserted. Press the user button to start the program. If all goes well, you'll hit the breakpoint with resp equal to 0xFF. Step out of the function, and you'll see that it returned false because 0xFF is interpreted as a negative number.
  • Add the compiler flag -funsigned-char to fix the problem.

See also discussion at https://developer.mbed.org/questions/62085/No-response-from-SD-card-on-FRDM-K64F/.

@aamcampbell
Copy link
Author

Also, I'm not quite comfortable submitting a pull request because I'm not familiar with the export scripts and I have no way of testing them. I assume I would add "-funsigned-char" to common_flags, but I wouldn't be completely sure.

@theotherjimmy theotherjimmy self-assigned this Jun 7, 2016
theotherjimmy added a commit to theotherjimmy/mbed that referenced this issue Jun 10, 2016
It would resolve ARMmbed#1525, but for the fact that KDS is used.
deepakvenugopal added a commit to deepakvenugopal/mbed-os that referenced this issue Feb 9, 2018
…changes from c9bf20f..43c7ec2

43c7ec2 Merge branch 'release_internal' into release_external
ed76459 Merge pull request ARMmbed#1558 from ARMmbed/IOTTHD-2195
8d3bcb7 Add new function to unit tests
56f66a4 Review correction
8b7d018 Move indirect queue size public API to net_interface
d877c9e Review corrections
c25e476 Remove Eclipse project files for external release
d51f442 Merge branch 'release_internal' into release_external
135c48d Increase Thread SED buffer size for big packets
70931a7 Fix indirect queue packet ordering
10e51a4 API for changing Thread SED parent buffer size
6122d24 dereference null value issue fixed. (ARMmbed#1557)
d1378dc Clear IPv6 neighbor cache in partition change (ARMmbed#1554)
7610e91 Child neighbor entry updates (ARMmbed#1550)
c727295 cleared neighbours with child address that are not ours (ARMmbed#1549)
80b4d72 Thread partition merge mode TLV change (ARMmbed#1546)
edd7599 RLOC was updated before clearing child info (ARMmbed#1547)
a666056 router short address set to 0xfffe for non routers (ARMmbed#1543)
759ab05 delete route set and link set entries for a router ID (ARMmbed#1540)
23a1265 REED advertisement handling (Thread spec 5.16.3): (ARMmbed#1535)
0a32cb4 added active and pending timestamps to child update response (ARMmbed#1533)
d0eec80 Fix error case memory leak (ARMmbed#1537)
da9860f Pending set after link sync (ARMmbed#1526)
ffa1569 Thread router network data update after link sync (ARMmbed#1530)
3b46d8d Fix defects found by coverity (ARMmbed#1529)
3a57101 Fix compiler warnings and update traces (ARMmbed#1523)
c288227 Add extension check for partition weight drop in parent selection (ARMmbed#1521)
d8dea28 network data cleared after router forms new partition (ARMmbed#1525)
44a85e5 removed router flagging for thread_management_server file (ARMmbed#1524)
1cbced9 Merge pull request ARMmbed#1520 from ARMmbed/IOTTHD-2105_2
3d07365 Review corrections to network data clearing
22a0375 Clear network data from lost children
ffd8517 added a new thread management function (ARMmbed#1519)
80af9cb Thread BR network data clearing (ARMmbed#1518)
5a6f6b5 thread nvm valgrind uninitialized data fix (ARMmbed#1517)
d5e2198 Add API for partition weighting set (ARMmbed#1513)
8811d6f multicast forwarding scope changed and address registration updated. (ARMmbed#1516)
c277384 printf to tr_info (ARMmbed#1515)
bb21264 Thread combined nvm test (ARMmbed#1507)
76f7725 Primary BBR fixes from interop (ARMmbed#1512)
12ed5ab FHSS unit test: fixed fhss mac interface test (cherry picked from commit 805eb42e4416b00cc018dc32dceb353d0b6c8bb6)
dd21ea9 Remvoed unnecessary trace print's.
cb6e78b FHSS unit test: fixed fhss beacon tasklet test (cherry picked from commit abe6d671b058f4f069741eab24d51e4d62d550b0)
237b3d4 Fhss info print (ARMmbed#1486)
0f39a47 FHSS: Do not update synch monitor right after superframe change (cherry picked from commit 99d50ad9d7f8dad80f10c2a4303f4e75ab31a3c2)
c9a098f Fixed Timeoout force which actually never generate timeout.
957c7fb Pana server and client update:
ae230e5 FHSS: Update Beacon synch info in critical state
84bd8a4 FHSS: Synchronization must be done in critical state
fb1b163 Pan coordinator blacklist update
39fe6ba Added missing HAVE_RPL compiler flag
16a1bc5 MLE bootsrap and message timeout update
eeb2d39 enable BBR to support multicast registration in non   commercial networks (ARMmbed#1509)
4ea2bf8 uri modified. (ARMmbed#1510)
f443853 timeout corrected for neighbour entry (ARMmbed#1508)
ea93c1f Thread dev conf taken use (ARMmbed#1503)
5d5b239 bug fix in bbr start (ARMmbed#1505)
8dbd521 commented a trace. (ARMmbed#1504)
145dbdf device conf copy fix (ARMmbed#1502)
f60268f eid&random mac moved to device conf struct (ARMmbed#1497)
df18635 Let MAC choose address when mesh forwarding
42f916b fixed BBR stop to remove network data and routing information (ARMmbed#1500)
e058c2a pbbr changes (ARMmbed#1499)
1ece307 Merge pull request ARMmbed#1485 from ARMmbed/merge_release_back
dda8164 thread address handling updated. (ARMmbed#1496)
1dc21a1 thread extension fixes. (ARMmbed#1495)
543fe98 Merge branch 'release_internal'
bade70e Dua req changes (ARMmbed#1494)
1979df8 added status to MLR response and implemeted BMLR.ntf multicast. (ARMmbed#1492)
1807c01 mle class initialisation (ARMmbed#1488)
d809831 Merge pull request ARMmbed#1479 from ARMmbed/merge_release_to_master
bce812d Update license to Thread test file (ARMmbed#1483)
aaa4b1f Revert eclipse file removal

git-subtree-dir: features/nanostack/FEATURE_NANOSTACK/sal-stack-nanostack
git-subtree-split: 43c7ec2
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

No branches or pull requests

5 participants