Clean up unused variables from HardwareSerial.cpp

[ribbons]

This pull request cleans up a couple of unused variables in HardwareSerial.cpp to improve clarity.

[matthijskooijman]

This commit is unfortunately wrong - the result of these variables isn't used, but the side effect of reading the UDR register is that the byte is removed from the FIFO, which is needed. If this line is removed, and a buffer flow occurs, I think that the RX complete interrupt will then continue to occur indefinately...

Assuming that your goal is to silence the compilers warning, you might need to find some other way to keep the read, but get rid of the warning...

Regarding the other variable you are removing, I already have a commit for that in #1711, which will hopefully be merged soon.

[cmaglie]

Writing just

UDR0;

instead of

unsigned char c = UDR0;

should do the trick.

@matthijskooijman
I've your serial patch in my queue, I'll review it soon.

C

[ribbons]

Oops! Thanks to both of you for your comments. I'll update this tomorrow to take out the current_config removal handled in #1711 and to update the c removal to use the solution proposed by @cmaglie.

[matthijskooijman]

@ribbons, if you do that, best base your changes on top of my #1711, which also touches related code.

[ribbons]

Commit updated as promised.

@matthijskooijman - Ah yes, I see what you mean now I've looked more closely at your changes. However (being fairly new to this) I'm not sure what is the correct way to achieve this - should I merge the commits from your PR into this branch?

[matthijskooijman]

It's probably best if you put your change(s) on top of my change, instead of merging. To do so, you should fetch my changes and create a local branch containing them:

git fetch https://github.com/arduino/Arduino.git refs/pull/1711/head
git checkout -b new_branch FETCH_HEAD

This puts you on a local branch called "new_branch", containing my changes. Now, add any changes you want (you could cherry-pick them, but IIRC I moved code so much that you'll probably want to redo your change from scratch).

When making a pull-request, it seems you can even tell github to use "refs/pull/1711/head" (i.e. my pullreq) as a base (you'll have to manually enter that string, it's not offered in the list). Not sure what this means for the way the pullrequest is merged (I don't think github can know where the pullrequest should be merged to), but that's something to sort out later (the devs can always just manually pull the commit).

[ribbons]

Alternatively, shall I wait until #1711 is merged and then re-implement my changes on top of that to save having to open a new PR?

[ribbons]

As proposed in my previous comment, I've updated this to remove the unused c variable (now in HardwareSerial_private.h) now that #1711 has been merged.

[matthijskooijman]

Did you happen to compare the hex or disassembly output with and without this change? I'd expect them to be equal, but it would be good to doublecheck.

[ribbons]

@matthijskooijman Good thinking - I'll do that tomorrow evening and report back...

[cmaglie]

Just checked, the output is exactly the same. Thanks!

[ribbons]

Many thanks both for your help - much appreciated.