Page MenuHomeFreeBSD

Fix some clang warnings.
ClosedPublic

Authored by araujo on Jun 3 2015, 7:59 AM.
Tags
None
Referenced Files
Unknown Object (File)
Apr 27 2017, 1:02 PM
Unknown Object (File)
Apr 15 2017, 4:03 PM
Unknown Object (File)
Apr 13 2017, 1:21 AM
Unknown Object (File)
Apr 8 2017, 9:49 PM
Unknown Object (File)
Mar 28 2017, 10:46 PM
Unknown Object (File)
Mar 10 2017, 11:16 PM
Unknown Object (File)
Feb 10 2017, 1:59 PM
Unknown Object (File)
Feb 7 2017, 12:20 AM
Subscribers

Details

Summary
  1. At revision r247852 @mm accidentally we forgot to print out the obj too.
  2. Fix some void function that don't really need to return.
  3. Also for some global variables, declare it as static.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

araujo retitled this revision from to Fix some clang warnings..
araujo updated this object.
araujo edited the test plan for this revision. (Show Details)
araujo added reviewers: rodrigc, delphij.
delphij requested changes to this revision.Jun 7 2015, 4:58 AM
delphij edited edge metadata.

Please note that the two comments inline, I think they are wrong and should be reverted. The rest part of the proposed patch looks good to me.

cddl/contrib/opensolaris/cmd/zdb/zdb.c
182 ↗(On Diff #5906)

This is wrong. usage() is supposed to be an exit point.

201 ↗(On Diff #5906)

This is also wrong, similar to usage() fatal is supposed to be a __dead exit.

This revision now requires changes to proceed.Jun 7 2015, 4:58 AM
araujo edited edge metadata.

As @delphij pointed out, I have reverted those changes related with the 'exit(1);' case.

Adding Matt to reviewer so he gets notification. I think we should upstream most of the changes to Illumos -- Marcelo, can you create an Illumos ticket for this? I can create a review request there too but since you wrote the patch it's better that you get involved too :)

cddl/contrib/opensolaris/cmd/zdb/zdb.c
96 ↗(On Diff #6013)

These changes should be upstreamed to Illumos.

1492 ↗(On Diff #6013)

This should be upstreamed to Illumos.

1494 ↗(On Diff #6013)

I'm not sure about the style here -- I think if I was the author I would have a blank line on after line 1490, but it's not clear if it also matches Illumos code style.

1500 ↗(On Diff #6013)

This should be upstreamed too.

delphij edited edge metadata.

The revision seems good to me.

This revision is now accepted and ready to land.Jun 10 2015, 6:53 PM
cddl/contrib/opensolaris/cmd/zdb/zdb.c
95 ↗(On Diff #6013)

I don't see any references to this in zdb_il.c; can we make it "static" too?

1494 ↗(On Diff #6013)

doesn't matter much either way to me.

Adding Matt to reviewer so he gets notification. I think we should upstream most of the changes to Illumos -- Marcelo, can you create an Illumos ticket for this? I can create a review request there too but since you wrote the patch it's better that you get involved too :)

@delphij, Yes I can open a ticket there without any problem, I will do it today.

araujo edited edge metadata.

@mahrens thanks for the review, yes, that supposed to be static too.
Please check this new version of the patch.

Best,

This revision now requires review to proceed.Jun 11 2015, 1:29 AM

The revision seems good to me.

@delphij I had already open a BUG at Illumos project, here is the link: https://www.illumos.org/issues/5999

Thanks,

mahrens edited edge metadata.
This revision is now accepted and ready to land.Jun 11 2015, 3:03 PM
delphij edited edge metadata.