for-loop-copy vs std::copy 中的错误,我不明白



下面是一个非常简单的(从大多数模板代码中剥离)串行输出接口的fifo缓冲区。我决定自己写一个,因为a)我正在努力学习c++, b)容器适配器queue没有带来"快速放多"的功能。设施,为每个元素强制显式的push()。我知道许多现代编译器在许多情况下可能会优化这一点,但为了a)我想自己做这件事-请随意评论这个想法和任何您认为值得注意的风格/方法错误。

这个问题只是处理"quick -put-many"的内循环。put()。使用std::copy()变体编译,一切看起来都很好,但是使用我自己的插入版本(-DBUGGY),数据部分被破坏了。

#include <cstddef>
#include <array>
#include <vector>
#include <atomic>
#include <algorithm>
#include <type_traits>
#include <iterator>
#include <iostream>
#include <string>
#include <queue>
#include <chrono>

struct SerialBuffer
{
std::array<char,127> fifo{};
std::atomic<int8_t> hd = 0, tl = 0, vtl = 0;
int8_t space(void) // return free space in queue
{
volatile int8_t tmp = hd - vtl - 1;
if (tmp < 0) { tmp += 127; }
return tmp;
}
int8_t reserve(int8_t n) // move virtual tail at once, reserving a run of bytes at end
{
volatile int8_t new_vtl = vtl;
if (n <= space()) {
if (new_vtl - 127 + n >= 0) { vtl = new_vtl - 127 + n; }
else { vtl = new_vtl + n; }
return new_vtl;
}
return -1;
}
int8_t next(int8_t i) // advance index in queue
{
if (i >= 127 - 1) { return 0; }
return i + 1;
}
void confirm(void) // confirm the formerly reserved bytes as present in queue
{
tl = static_cast<int8_t>(vtl);
}

int8_t headroom(int8_t i) // return number bytes from queue index to queue end
{
return 127 - i;
}

template<typename iter_t>
bool put(iter_t it, int8_t n) // (source, number of bytes)
{
int8_t i = reserve(n);
if (i >= 0) {
int8_t j = std::min(n, headroom(i)); // maybe two consecutive insert-ranges: first from i to buffer end, rest from buffer start
#ifdef BUGGY
for (; i < 127; i++) {
fifo[i] = *it++;
}
for (i = 0; i < n-j; i++) {
fifo[i] = *it++;
}
#else
std::copy(it, it+j, fifo.begin()+i);
std::copy(it+j, it+n, fifo.begin());
#endif

confirm(); 
return true;
}
return false;
}

bool put(std::vector<char> v) { return put(v.cbegin(),v.size()); }
bool put(std::basic_string<char> v) { return put(v.cbegin(),v.size()); }

void dump(int8_t k = 127)
{
if (space() < k) { hd = static_cast<int8_t>(tl); }
else { hd = (hd + k) % 127; }
}

void print(void)
{
std::cout << "Head:" << (0+hd) << " Tail:" << (0+tl) << " VirtTail:" << (0+vtl) << std::endl;
for (int8_t i = hd; i != tl; i = next(i)) { std::cout << fifo[i]; }
std::cout << std::endl;
}
};

int main(void)
{
using namespace std::string_literals;

SerialBuffer fifo1;
auto tmp{"/uwb/x1/raw:123456789"s};
std::vector<char> x(tmp.cbegin(),tmp.cend());
std::queue<char,std::array<char,127>> fifo2;
for (auto _: {1,2,3}) {
for (int i=0; i < 10'000'000; i++) {
if (!fifo1.put(x)) fifo1.dump();
}
fifo1.print();   
}
} 

结果:

$ g++ bug.cpp --std=c++17 -O3 && ./a.exe
Head:52 Tail:115 VirtTail:115
/uwb/x1/raw:123456789/uwb/x1/raw:123456789/uwb/x1/raw:123456789
Head:104 Tail:103 VirtTail:103
/uwb/x1/raw:123456789/uwb/x1/raw:123456789/uwb/x1/raw:123456789/uwb/x1/raw:123456789/uwb/x1/raw:123456789/uwb/x1/raw:123456789
Head:28 Tail:70 VirtTail:70
/uwb/x1/raw:123456789/uwb/x1/raw:123456789
$ g++ bug.cpp --std=c++17 -O3 -DBUGGY && ./a.exe
Head:52 Tail:115 VirtTail:115
/uwb/x1/raw:123456789/uwb/x1/raw:123456789/uwb/x1/raw:123456789
Head:104 Tail:103 VirtTail:103
▒ե▒qс▒▒1▒3▒▒wb/x1/raw:123456789/uwb/x1/raw:123456789/uwb/x1/raw:123456789/uwb/x1/raw:123456789/uwb/x1/raw:123456789
Head:28 Tail:70 VirtTail:70
/uwb/x1/raw:123456789/uwb/x1/raw:123456789

正如你所看到的,有篡改的字节在第二运行。我不明白我在那些看似无害的for循环中犯了什么错误。

编辑:正如@yzt指出的,这是一个令人尴尬的简单逻辑错误。我首先写了一个(正确的)基于for的版本,然后更改为std::copy,然后在晚上太晚了,试图通过重写for循环来测量运行时差异,这次是错误的。不好意思,这是"不要承诺,运行不了就回家"的导数错误。正确的代码:

n -= j;
for (; j > 0; j--,i++) {
fifo[i] = *it++;
}
for (i = 0; i < n; i++) {
fifo[i] = *it++;
}

一个bug是,在你的第一个循环中,当你从0开始时(即使在第一次调用时也会发生),你将vector迭代器加127次,这显然超出了范围。

拷贝工作,而你的循环不起作用,因为你的循环和你正在做的std::copy调用正在做不同的事情。它们不会在相同的索引处开始或停止,因此它们不会复制相同的内容。

类似于副本的循环将是:

for (ptrdiff_t index = 0; index < j; ++index)
*(fifo.begin() + i + index) = *(it + index); // the exact way you dereference isn't important; could have used array subscripting
for (ptrdiff_t index = 0; index < n - j; ++index)
*(fifo.begin() + index) = *(it + j + index);

另外,我建议您使用比int8_t(例如int)更大的类型进行内部,中间计算。可能存在一些溢出或错误计算。

最新更新