Page MenuHomeFreeBSD
Differential D4720

Clean up dead code and fix a warning: unused-but-set-variable spotted by gcc4.9.
ClosedPublic
Actions

Authored by araujo on Dec 28 2015, 8:01 AM.
Tags
None
Referenced Files
Unknown Object (File)
Oct 19 2024, 9:33 AM
Unknown Object (File)
Oct 12 2024, 8:19 PM
Unknown Object (File)
Oct 10 2024, 8:00 AM
Unknown Object (File)
Oct 2 2024, 4:45 AM
Unknown Object (File)
Oct 2 2024, 2:42 AM
Unknown Object (File)
Oct 1 2024, 4:04 PM
Unknown Object (File)
Sep 30 2024, 9:08 AM
Unknown Object (File)
Sep 27 2024, 2:12 PM
View All Files
Subscribers
imp
ngie

Details

Reviewers
• rodrigc
ae
ngie
bapt
Commits
rS292972: Wrap using #ifdef 'notyet' those variables and statements not yet

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 1881
Build 1888: arc lint + arc unit

Event Timeline

araujo updated this revision to Diff 11737.Dec 28 2015, 8:01 AM
araujo retitled this revision from to Clean up dead code and fix a warning: unused-but-set-variable..
araujo updated this object.
araujo edited the test plan for this revision. (Show Details)
Herald added a subscriber: imp. · View Herald TranscriptDec 28 2015, 8:01 AM
araujo retitled this revision from Clean up dead code and fix a warning: unused-but-set-variable. to Clean up dead code and fix a warning: unused-but-set-variable spotted by gcc4.9..Dec 28 2015, 8:03 AM
araujo added reviewers: • rodrigc, bapt, ae.
araujo added a subscriber: ngie.Dec 28 2015, 8:07 AM
ae edited edge metadata.Dec 29 2015, 6:24 PM

I have not any strong objection about this. But I prefer the commented code should be left here. I added it to be compatible with current behavior, but in future we can change this.

ngie added a comment.Dec 29 2015, 7:30 PM
In D4720#100209, @ae wrote:

I have not any strong objection about this. But I prefer the commented code should be left here. I added it to be compatible with current behavior, but in future we can change this.

I think removing the code's ok, but maybe a better XXX comment should be put in so someone could fill in the blanks later (I ran into some code in lib/libc/rpc recently like this with unimplemented paths)?

araujo added a comment.Dec 30 2015, 1:39 AM
In D4720#100240, @ngie wrote:
In D4720#100209, @ae wrote:

I have not any strong objection about this. But I prefer the commented code should be left here. I added it to be compatible with current behavior, but in future we can change this.

I think removing the code's ok, but maybe a better XXX comment should be put in so someone could fill in the blanks later (I ran into some code in lib/libc/rpc recently like this with unimplemented paths)?

I agree with @ngie, @ae could you give me any input about how we could add some comments there? Maybe we can remove that commented code, and insert at the comments the revision that add that code and an explanation about what that should do.

what do you think @ae?

ae added inline comments.Dec 30 2015, 7:37 AM
sys/net/if_gre.c
718

XXX: The current implementation uses the key only for outgoing packets. But we can check the key value here, or even in the encapcheck function.

araujo updated this revision to Diff 11787.EditedDec 30 2015, 7:48 AM
araujo edited edge metadata.

Address @ae comments, and for now we don't remove the code.
We will still have the gcc-4.9 warning, but for those that are cleaning it up, they will know by
the comment, it is something not implemented yet.

ngie added inline comments.Dec 30 2015, 8:03 AM
sys/net/if_gre.c
721

Please add a newline before the XXX per style(9).

araujo updated this revision to Diff 11788.Dec 30 2015, 8:05 AM

Respect style(9) by @ngie.

ngie added a comment.Dec 30 2015, 8:09 AM

Sidenote: I'm fine (personally) with removing the variables or commenting out the code under another #ifdef (e.g. KEY_SUPPORT_NOTYET), as long as the comments are there documenting the item that needs to be implemented (so two birds could be killed with one stone)

sys/net/if_gre.c
722

Please add a space between the XXX: and The.

araujo added a comment.Dec 30 2015, 8:11 AM
In D4720#100407, @ngie wrote:

Sidenote: I'm fine (personally) with removing the variables or commenting out the code under another #ifdef (e.g. KEY_SUPPORT_NOTYET), as long as the comments are there documenting the item that needs to be implemented (so two birds could be killed with one stone)

I would prefer a #ifdef too, the meaning of this patch is to lower the compiler warnings. I will address @ngie suggestion together with the 'space'.

araujo updated this revision to Diff 11789.Dec 30 2015, 8:28 AM

Wrap using #ifdef notyet those variables and statements not yet implemented.
Suggested by @ngie.

@ae what do you think about it?

araujo updated this revision to Diff 11790.Dec 30 2015, 8:29 AM

Cosmetic fix, one tab missed.

ngie accepted this revision.Dec 30 2015, 8:56 AM
ngie added a reviewer: ngie.

LGTM. @ae should sign off too.

This revision is now accepted and ready to land.Dec 30 2015, 8:56 AM
bapt accepted this revision.Dec 30 2015, 10:40 AM
bapt edited edge metadata.
ae accepted this revision.Dec 30 2015, 10:55 AM
ae edited edge metadata.
Closed by commit rS292972: Wrap using #ifdef 'notyet' those variables and statements not yet (authored by araujo). · Explain WhyDec 31 2015, 2:01 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
sys/
net/
22 lines

Diff 11790

View Options

sys/net/if_gre.c

Loading...