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.

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

common: include: Redo some includes for FreeBSD #15337

Merged
merged 2 commits into from May 29, 2017

Conversation

Projects
None yet
3 participants
@wjwithagen
Contributor

wjwithagen commented May 27, 2017

  • During include cleanup just a too much was removed for FreeBSD
    to get things compiled.
    This redoes some of the includes.

Tracker: http://tracker.ceph.com/issues/19883
Signed-off-by: Willem Jan Withagen wjw@digiware.nl

@wjwithagen wjwithagen changed the title from build/include: Redo som includes for FreeBSD to build/include: Redo some includes for FreeBSD May 27, 2017

@wjwithagen wjwithagen requested a review from joscollin May 27, 2017

@joscollin joscollin added the needs-qa label May 27, 2017

@joscollin
  1. Could you please verify whether this PR follows the naming conventions for the PR Title and Commit Title ? They need change.
    Please refer: https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#3-describe-your-changes

  2. My PRs for "Removing Redundant Headers" affects only the files inside src/common/ directory. But this PR contains "erasure-code" and "test" submodules and these are not re-added for sure. I usually create a separate PR for each submodule and especially the ones that has Separate logical changes (here: newly added, not reverted). You could consider that if it is not a hard for you.

  3. If this PR does a revert, then please add the Tracker URL in the Commit Message and in the PR Description.

  4. Update the tracker with the PR URLs.

#include <unistd.h>
#if defined(__FreeBSD__)

This comment has been minimized.

@joscollin

joscollin May 28, 2017

Member

Is there any problem making it similar ? I mean putting an #ifdef, which is similar to the other checks (linux)

This comment has been minimized.

@wjwithagen

wjwithagen May 28, 2017

Contributor

@joscollin
I've been using #if defined() for all my submissions.
This will allow me an easy grep to find all typical FreeBSD stuff.
And in the whole code there is a mismatch of all the possible forms.

#include <arpa/inet.h>
#include <ifaddrs.h>
#include <stdlib.h>
#include <string.h>
#if defined(__FreeBSD__)
#include <sys/types.h>

This comment has been minimized.

@joscollin

joscollin May 28, 2017

Member

I have just noticed that this is newly added. But It is fine, if it is necessary in FreeBSD.

@@ -15,6 +15,9 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#if defined(__FreeBSD__)
#include <sys/wait.h>

This comment has been minimized.

@joscollin

joscollin May 28, 2017

Member

I have just noticed that this is newly added. But It is fine, if it is necessary in FreeBSD.

This comment has been minimized.

@wjwithagen

wjwithagen May 28, 2017

Contributor

@joscollin
I'm not quite sure why these would be newly added.
The older version I have does have this include.
It even works without the #if

@wjwithagen wjwithagen changed the title from build/include: Redo some includes for FreeBSD to common: include: Redo some includes for FreeBSD May 28, 2017

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented May 28, 2017

@joscollin

I'm undoing more than just your reorganisation. Hence the erasure code patch.
I can make 2 different PRs if you prefer.

And I'll do the tracker administration.

wjwithagen added some commits May 27, 2017

build/include: Redo som includes for FreeBSD
- During include cleanup just a too bit much was removed for FreeBSD
  to get things compiled.
  This redoes some of the includes.

Tracker: http://tracker.ceph.com/issues/19883
Signed-off-by: Willem Jan Withagen <wjw@digiware.nl>
erasure-code/isa/ErasureCodePluginIsa.cc: fixup includes
 - Buffer.h is needed to prevent Clang seriously complaining
   about missing and wrong forward declarations of
	ceph::buffer:ptr
   including buffer_fwd.h does not help.

Signed-off-by: Willem Jan Withagen <wjw@digiware.nl>

looks good

@liewegas liewegas merged commit 0f803c5 into ceph:master May 29, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment