Skip to content

ATLEDGE-553: Modify library examples header info #173

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.

Sign up for GitHub

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

Closed
wants to merge 7 commits into from
Closed

ATLEDGE-553: Modify library examples header info #173

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7969282
ATLEDGE-553: Modify library examples header info
Apr 29, 2016
b5a10ce
Moved Copyright terms down to bottom of same file instead of in a sep…
May 4, 2016
db5e3be
ATLEDGE-553: Modify library examples header info. Update Copyright 2016
May 25, 2016
7a2670a
ATLEDGE-553: Modify library examples header info. Update Copyright 2016
May 25, 2016
2c7f034
ATLEDGE-586: String.compareTo() does not compute the same as Uno for …
May 26, 2016
850b8b3
Backed out fix for strcmp() bug as sugguested by team. To be investig…
May 28, 2016
5053859
Merge branch 'master' of https://github.com/bokibi/corelibs-arduino101
May 28, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion cores/arduino/WString.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,13 @@ int String::compareTo(const String &s) const
if (buffer && len > 0) return *(unsigned char *)buffer;
return 0;
}
return strcmp(buffer, s.buffer);
const char *p1 = buffer;
const char *p2 = s.buffer;

while (*p1 == *p2++)
if ('\0' == *p1++)
return 0;
return (*(const char *)p1 - *(const char *)(p2 - 1));
Copy link
Contributor

@eriknyquist eriknyquist May 26, 2016

Choose a reason for hiding this comment

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

Sneaking in a functional change ;)
I'm happy with this, usually not a fan of re-implementing std routines but in this case it does appear that
"strcmp" is indeed not behaving as advertised. I'll fix the conflict and merge it.

This seems odd to me since we are not using any home-made versions of strcmp, it just comes from the std. C++ lib (which, admittedly, comes with our toolchain). If there is an error in the libs shipped with our toolchain, I'd like to find it-- when was the first time you noticed this issue?

Copy link
Contributor

@calvinatintel calvinatintel May 26, 2016

Choose a reason for hiding this comment

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

Wuh.. this is not okay. I see a few more code changes in the PR. Perhaps the PR needs to be rebased to the tip?

Copy link
Contributor

@eriknyquist eriknyquist May 26, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Oh yeah you're right, there are more functional changes that I missed. OK, so @bokibi can you please remove functional changes from this PR? leave header changes only, like the title suggests.

That said, I believe your "strcmp" re-implementation is a valid fix, and I would like to merge it also if we cannot fix the toolchain. Please make a new pull request for each functional change. And, like @calvinatintel says you'll probably need to rebase.

Copy link
Contributor

@SidLeung SidLeung May 26, 2016

Choose a reason for hiding this comment

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

Agree with Calvin. It may require more work. However, for future referencing and categorizing, it is desirable to separate out issues into individual reports. It will also ensure the discussion, analysis of the issues will not be mixed together.

Copy link
Author

@bokibi bokibi May 26, 2016 •
edited by calvinatintel
Loading

Choose a reason for hiding this comment

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

​Agree. I will back out the change for strcmp issue. Erik, the issue was
found by Janet Y Scholar and assigned to me.

}

unsigned char String::equals(const String &s2) const
Expand Down