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

rgw: migrate atomic_t to std::atomic<> #15001

Merged
merged 1 commit into from May 19, 2017

Conversation

Projects
None yet
4 participants
@chardan
Contributor

chardan commented May 8, 2017

Signed-off-by: Jesse Williamson jwilliamson@suse.de

@liewegas

This comment has been minimized.

Member

liewegas commented May 8, 2017

needs rebsae

@@ -37,6 +38,8 @@
#include "rgw_usage.h"
#include "rgw_replica_log.h"
#include <atomic>

This comment has been minimized.

@mikulely

mikulely May 9, 2017

Contributor

Why we need to include this twice?

This comment has been minimized.

@chardan

chardan May 10, 2017

Contributor

We do not! Fixed. Thank you!

This comment has been minimized.

@mikulely

mikulely May 10, 2017

Contributor

Thx.

@chardan

This comment has been minimized.

Contributor

chardan commented May 10, 2017

@@ -4,12 +4,6 @@
#ifndef CEPH_OBJEXP_H
#define CEPH_OBJEXP_H
#include <errno.h>

This comment has been minimized.

@mikulely

mikulely May 10, 2017

Contributor

one more nit, it seems that we're following the convention including std headers before self-defined headers.

This comment has been minimized.

@chardan

chardan May 10, 2017

Contributor

@mikulely We don't seem to be overly consistent about it. I've occasionally been moving standard headers together and after "our" headers, since this is one way to help ensure that our headers are including what they need. OTOH, it's true that if we aren't really following through on that the benefit isn't entirely palpable. If you feel strongly about it, I can move them back, but I noticed that taking out our atomic header is causing issues like needing to rewrite dout.

@liewegas

This comment has been minimized.

Member

liewegas commented May 10, 2017

@chardan

This comment has been minimized.

Contributor

chardan commented May 10, 2017

@liewegas Ok, noted.
@mikulely Will do, thanks.

@cbodley

This comment has been minimized.

Contributor

cbodley commented May 17, 2017

@chardan not sure why jenkins didn't catch this, but it's failing to build rgw_loadgen_process.cc:

ceph/src/rgw/rgw_loadgen_process.cc: In member function ‘virtual void RGWLoadGenProcess::run()’:
ceph/src/rgw/rgw_loadgen_process.cc:50:42: error: no matching function for call to ‘RGWLoadGenProcess::gen_request(const char [4], std::__cxx11::string&, int, std::atomic<bool>*)’
gen_request("PUT", bucket, 0, &failed);
^
In file included from ceph/src/rgw/rgw_frontend.h:11:0,
from ceph/src/rgw/rgw_loadgen_process.cc:10:
ceph/src/rgw/rgw_process.h:186:8: note: candidate: void RGWLoadGenProcess::gen_request(const string&, const string&, int, std::atomic<long int>*)
void gen_request(const string& method, const string& resource,
^
ceph/src/rgw/rgw_process.h:186:8: note: no known conversion for argument 4 from ‘std::atomic<bool>*’ to ‘std::atomic<long int>*’

rgw: migrate atomic_t to std::atomic<>
Signed-off-by: Jesse Williamson <jwilliamson@suse.de>
@chardan

This comment has been minimized.

Contributor

chardan commented May 18, 2017

@cbodley ...if at first you don't succeed... :-)

@cbodley

This comment has been minimized.

@cbodley cbodley merged commit e546f46 into ceph:master May 19, 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