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

Bug in a for-loop-copy vs std::copy which I don't understand

下面是一个非常简单(从大部分模板代码中剥离)的串行输出接口的 fifo 缓冲区。我决定自己编写,因为 a) 我正在尝试学习 C++,b) 容器适配器 queue 没有带来“快速放置许多”功能,强制执行显式 push() 对于每个元素。我知道许多现代编译器在许多情况下可能会对此进行优化,但为了 a) 我想自己做这个 - 请随意评论这个想法和任何你认为值得注意的 style/methodic 错误。

然而,这个问题只是处理“quickly-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++;
  }

一个错误是,在您的第一个循环中,当您从 0 开始时(即使在第一次调用时也会发生),您会将向量迭代器递增 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)进行内部中间计算。那里可能潜伏着一些溢出或错误计算。