Skip to content

Conversation

@michalpasztamobica
Copy link
Contributor

Description

Fixes #10198
Macro which restricted compilation to GCC_ARM is removed.
Existing read_write() test is amended to call stat() and check that correct size is returned.

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@SeppoTakalo
@geky
@kjbracey-arm

Macro which restricted compilation to GCC_ARM is removed.
Existing read_write() test is amended to call stat() and check that correct size is returned.
@ciarmcom ciarmcom requested review from a team, SeppoTakalo, geky and kjbracey August 9, 2019 13:00
@ciarmcom
Copy link
Member

ciarmcom commented Aug 9, 2019

@michalpasztamobica, thank you for your changes.
@kjbracey-arm @SeppoTakalo @geky @ARMmbed/mbed-os-storage @ARMmbed/mbed-os-maintainers please review.

Copy link
Contributor

@kjbracey kjbracey left a comment

Choose a reason for hiding this comment

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

Can you check if this breaks ARMC5? You might want to have a #ifndef __CC_ARM there for that.

Despite lack of official support or testing, if it's easy to avoid breakage, we should.

@michalpasztamobica
Copy link
Contributor Author

@kjbracey-arm , I checked that the proposed changes compile fine with:

Product: DS-5 Ultimate Edition 5.29.2
Component: ARM Compiler 5.06 update 3 (build 300)

and the test (as modified in this review) passed.

@kjbracey
Copy link
Contributor

Cool. I wonder why it was originally thought not to work on ARMCC then?

@michalpasztamobica
Copy link
Contributor Author

To my understanding the reasons were twofold: 1) not enough testing to be sure that this worked 2) even if the testing was there and indicated an issue a couple of years ago, then both IAR and ARM compiler improved since then - the #ifdef is 3 years old and might have been copied from an even older location, unchanged.

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 12, 2019

CI restarted

@mbed-ci
Copy link

mbed-ci commented Aug 12, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FATFileSystem::stat() function broken with IAR

6 participants