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

core: Prefix /proc/ with FreeBSD emulation #14290

Merged
merged 1 commit into from Apr 11, 2017

Conversation

Projects
None yet
3 participants
@wjwithagen
Contributor

wjwithagen commented Apr 2, 2017

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

@wjwithagen wjwithagen requested a review from tchaikov Apr 2, 2017

@@ -14,8 +14,13 @@
#include "acconfig.h"
#define PROCPREFIX ""

This comment has been minimized.

@tchaikov

tchaikov Apr 3, 2017

Contributor

could just put

#if defined(__FREEBSD__)
#define PROCPREFIX "/compat/linux"
#else
#define PROCPREFIX
#endif

This comment has been minimized.

@wjwithagen

wjwithagen Apr 3, 2017

Contributor

@tchaikov
I first considered doing that.

But I liked the definition under one FreeBSD clause better.
And it allows for other OSes to do the same.

But if you feel strong about this, I don't mind changing

This comment has been minimized.

@tchaikov

tchaikov Apr 3, 2017

Contributor

okay how about

#if defined(__linux__)
#define PROCPREFIX
#endif

#if defined(__FreeBSD__)
#define PROCPREFIX "/compat/linux"
...

as the proc fs only exists on linux, and its counterpart linprocfs only on FreeBSD. i just don't really like the unnecessarily defined empty string prefix for linux and #undef

This comment has been minimized.

@wjwithagen

wjwithagen Apr 3, 2017

Contributor

@tchaikov
I'm not picky. :)
So and ìf` per OS is fine with me too.

@@ -33,7 +34,7 @@ TEST(Arch, all)
#if (__arm__ || __aarch64__ || __x86_64__) && __linux__
char flags[FLAGS_SIZE];
FILE *f = popen("grep '^\\(flags\\|Features\\)[ ]*:' "
"/proc/cpuinfo | head -1", "r");
PROCPREFIX "/proc/cpuinfo | head -1", "r");

This comment has been minimized.

@tchaikov

tchaikov Apr 3, 2017

Contributor

we are checking if __linux__ here already. so no need to change here.

This comment has been minimized.

@wjwithagen

wjwithagen Apr 3, 2017

Contributor

@tchaikov
Yes. My challenge is that the ARM code did work because ARM has enough FreeBSD support to compile. So I was going to figure out later on if this was really a Linux only case.
And then I would not have forgotten the PROCPREFIX.
But if you prefer me to take 'm out, I have no problem with that.

This comment has been minimized.

@wjwithagen

wjwithagen Apr 3, 2017

Contributor

@tchaikov
Have taken this part out of the PR, looked more deeply into it.
And I needed I'll have to fix it in the future.

@@ -16,6 +16,7 @@
#include <stdio.h>
#include "include/compat.h"

This comment has been minimized.

@tchaikov

tchaikov Apr 3, 2017

Contributor

ditto.

@@ -14,8 +14,13 @@
#include "acconfig.h"
#define PROCPREFIX ""

This comment has been minimized.

@tchaikov

tchaikov Apr 3, 2017

Contributor

okay how about

#if defined(__linux__)
#define PROCPREFIX
#endif

#if defined(__FreeBSD__)
#define PROCPREFIX "/compat/linux"
...

as the proc fs only exists on linux, and its counterpart linprocfs only on FreeBSD. i just don't really like the unnecessarily defined empty string prefix for linux and #undef

@@ -1,4 +1,4 @@
#include "arch/probe.h"
"#include "arch/probe.h"

This comment has been minimized.

@tchaikov

tchaikov Apr 3, 2017

Contributor

why the leading "?

This comment has been minimized.

@wjwithagen

wjwithagen Apr 3, 2017

Contributor

@tchaikov
Typo from removing compat.h
Fixed

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Apr 5, 2017

@tchaikov
This is holding at "changes requested"
But as far a I know nothing is outstanding any more?

#if defined(__FreeBSD__)
// FreeBSD support the linux procfs with its compatibility module

This comment has been minimized.

@tchaikov

tchaikov Apr 5, 2017

Contributor

supports
compatibility

This comment has been minimized.

@wjwithagen

wjwithagen Apr 5, 2017

Contributor

@tchaikov
Fixed typos. Thanx

core: Prefix /proc/ with FreeBSD emulation
Signed-off-by: Willem Jan Withagen <wjw@digiware.nl>

@yuriw yuriw merged commit 612f806 into ceph:master Apr 11, 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