Skip to content
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

Linux support for positional audio plugins #2507

Merged
merged 1 commit into from Aug 11, 2016

Conversation

@davidebeatrici
Copy link
Member

commented Aug 10, 2016

No description provided.

#define MUMBLE_PLUGIN_LINUX_H_

#ifndef PTR_TYPE_CONCRETE
#define PTR_TYPE_CONCRETE PTR_TYPE

This comment has been minimized.

Copy link
@mkrautz

mkrautz Aug 10, 2016

Member
# define PTR_TYPE_CONCRETE PTR_TYPE
#endif

#define _USE_MATH_DEFINES
#define NOMINMAX

This comment has been minimized.

Copy link
@mkrautz

mkrautz Aug 10, 2016

Member

Drop.

#define PTR_TYPE_CONCRETE PTR_TYPE
#endif

#define _USE_MATH_DEFINES

This comment has been minimized.

Copy link
@mkrautz

mkrautz Aug 10, 2016

Member

Drop.

#define _USE_MATH_DEFINES
#define NOMINMAX

typedef unsigned char BYTE;

This comment has been minimized.

Copy link
@mkrautz

mkrautz Aug 10, 2016

Member

Drop.

return content;
}

static inline PTR_TYPE_CONCRETE getModuleAddr(pid_t pid, std::string mod) {

This comment has been minimized.

Copy link
@mkrautz

mkrautz Aug 10, 2016

Member

Please ensure this function is indented correctly. It uses spaces sometimes. It should use tabs throughout.

pModule = 0;

if (! pids.empty()) {
std::multimap<std::wstring, unsigned long long int>::const_iterator iter = pids.find(procname);

This comment has been minimized.

Copy link
@mkrautz

mkrautz Aug 10, 2016

Member

Unindent one level.

@@ -0,0 +1,14 @@
// Copyright 2005-2016 The Mumble Developers. All rights reserved.

This comment has been minimized.

Copy link
@mkrautz

mkrautz Aug 10, 2016

Member

Please call this file mumble_plugin_linux_64bit.h. It has no x86_64 specific content. Only the pointer size is relevant.


#ifndef MUMBLE_PLUGIN_LINUX_X64_H_
#define MUMBLE_PLUGIN_LINUX_X64_H_

This comment has been minimized.

Copy link
@mkrautz

mkrautz Aug 10, 2016

Member
#include <stdint.h>
#ifndef MUMBLE_PLUGIN_LINUX_X64_H_
#define MUMBLE_PLUGIN_LINUX_X64_H_

typedef unsigned long long procptr64_t;

This comment has been minimized.

Copy link
@mkrautz

mkrautz Aug 10, 2016

Member
typedef uint64_t procptr64_t;

#ifndef MUMBLE_PLUGIN_LINUX_X86_H_
#define MUMBLE_PLUGIN_LINUX_X86_H_

This comment has been minimized.

Copy link
@mkrautz

mkrautz Aug 10, 2016

Member
#include <stdint.h>
#ifndef MUMBLE_PLUGIN_LINUX_X86_H_
#define MUMBLE_PLUGIN_LINUX_X86_H_

typedef unsigned long procptr32_t;

This comment has been minimized.

Copy link
@mkrautz

mkrautz Aug 10, 2016

Member
typedef uint32_t procptr32_t;
static inline PTR_TYPE_CONCRETE getModuleAddr(pid_t pid, std::string mod) {
std::string mapsFn = std::string("/proc/") + std::to_string(static_cast<unsigned long long>(pid)) + std::string("/maps");
std::string maps = readAll(mapsFn);

This comment has been minimized.

Copy link
@mkrautz

mkrautz Aug 10, 2016

Member

Add:

if (maps.size() == 0) {
    return 0;
}
}
}

return 1;

This comment has been minimized.

Copy link
@mkrautz

mkrautz Aug 10, 2016

Member

This is very wrong.
This is a failure case, in which case the function should return the null pointer, 0.

std::string modnameNonWide(modname.begin(), modname.end());
pModule = getModuleAddr((modnameNonWide.size() > 0) ? modnameNonWide : procnameNonWide);

if (!pModule) {

This comment has been minimized.

Copy link
@mkrautz

mkrautz Aug 10, 2016

Member

Use if (pModule == 0).

@@ -52,6 +47,10 @@ static inline PTR_TYPE_CONCRETE getModuleAddr(pid_t pid, std::string mod) {
std::string mapsFn = std::string("/proc/") + std::to_string(static_cast<unsigned long long>(pid)) + std::string("/maps");
std::string maps = readAll(mapsFn);

if (maps.size() == 0) {

This comment has been minimized.

Copy link
@mkrautz

mkrautz Aug 10, 2016

Member

Bad indentation


if (iter != pids.end())
pPid = static_cast<pid_t>(iter->second);
else
pPid = 0;
} else {
pPid = 0;
pPid = 0;

This comment has been minimized.

Copy link
@mkrautz

mkrautz Aug 10, 2016

Member

Bad indentation?

// seek to pathname
do {
ch = ss.get();
if (ch == EOF) {

This comment has been minimized.

Copy link
@mkrautz

mkrautz Aug 11, 2016

Member

Bad indentation.

// seek to inode
do {
ch = ss.get();
if (ch == EOF) {

This comment has been minimized.

Copy link
@mkrautz

mkrautz Aug 11, 2016

Member

Bad indentation.

if (pathname.size() > lastSlash + 1) {
std::string basename = pathname.substr(lastSlash + 1);
if (basename == mod) {
unsigned long long addr = std::stoull(baseaddr, 0, 16);

This comment has been minimized.

Copy link
@mkrautz

mkrautz Aug 11, 2016

Member

Please use

unsigned long addr = strtoul(baseaddr.c_str(), NULL, 16);

I am not sure if this will need a cast due to the return type of this function.

This comment has been minimized.

Copy link
@mkrautz

mkrautz Aug 11, 2016

Member

(This is to support C++03)

}

static inline PTR_TYPE_CONCRETE getModuleAddr(pid_t pid, std::string mod) {
std::string mapsFn = std::string("/proc/") + std::to_string(static_cast<unsigned long long>(pid)) + std::string("/maps");

This comment has been minimized.

Copy link
@mkrautz

mkrautz Aug 11, 2016

Member

Please use:

std::stringstream ss;
ss << std::string("/proc/");
ss << static_cast<unsigned long>(pid);
ss << std::string("/maps");
std::string mapsFn = ss.get();

This comment has been minimized.

Copy link
@mkrautz

mkrautz Aug 11, 2016

Member

(This is to support C++03)

pPid = 0;
}

#endif

This comment has been minimized.

Copy link
@mkrautz

mkrautz Aug 11, 2016

Member

Missing newline at the end of the file.

@mkrautz

This comment has been minimized.

Copy link
Member

commented Aug 11, 2016

LGTM.

@mkrautz

This comment has been minimized.

Copy link
Member

commented Aug 11, 2016

But we need to squash this.

@mkrautz

This comment has been minimized.

Copy link
Member

commented Aug 11, 2016

LGTM.

@mkrautz mkrautz merged commit d364932 into mumble-voip:master Aug 11, 2016

@davidebeatrici davidebeatrici deleted the davidebeatrici:linux-plugin branch Aug 11, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.