laudrup / boost-wintls

Native Windows TLS stream wrapper for use with Asio
https://wintls.dev
Boost Software License 1.0
55 stars 13 forks source link

In sspi_handshake::operator()(), extra buffer handling section should have std::copy, not std::move #51

Closed mscottmueller closed 2 years ago

mscottmueller commented 2 years ago
  const auto extra_data_begin = input_data_.begin() + previous_size - extra_size;
  const auto extra_data_end = input_data_.begin() + previous_size;

  std::move(extra_data_begin, extra_data_end, input_data_.begin());

Should be

  const auto extra_data_begin = input_data_.begin() + previous_size - extra_size;
  const auto extra_data_end = input_data_.begin() + previous_size;

  std::copy(extra_data_begin, extra_data_end, input_data_.begin());

https://github.com/laudrup/boost-wintls/blob/372a82bfa37b157914b94a34e09dd9b84dff8f40/include/boost/wintls/detail/sspi_handshake.hpp#L165

laudrup commented 2 years ago

@mscottmueller Thanks a lot for your interest in this project.

Not that I think you're wrong in any way but do you think you could explain why std::move is incorrect in this case?

I remember being quite certain that the regions couldn't overlap but I could very well be wrong.

You are also very welcome to open a pull request with the explanation in the commit message.

Thanks.

mscottmueller commented 2 years ago

Oh I see. I was confused because #include was not in place. Yes, std::move should be okay.

Best Regards,

M. Scott Mueller

On Jun 1, 2022, at 12:56 PM, Kasper Laudrup @.***> wrote:

 CAUTION - This message originated outside AVEVA

@mscottmuellerhttps://github.com/mscottmueller Thanks a lot for your interest in this project.

Not that I think you're wrong in any way but do you think you could explain why std::move is incorrect in this case?

I remember being quite certain that the regions couldn't overlap but I could very well be wrong.

You are also very welcome to open a pull request with the explanation in the commit message.

Thanks.

— Reply to this email directly, view it on GitHubhttps://github.com/laudrup/boost-wintls/issues/51#issuecomment-1144072359, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADHPESEWAK54WR7BPQIBBGLVM6567ANCNFSM5XRYBH7A. You are receiving this because you were mentioned.Message ID: @.***>


AVEVA Group plc is registered in England at High Cross, Madingley Road, Cambridge, England CB3 0HB. Number 2937296.

laudrup commented 2 years ago

@mscottmueller

The algorithm header should probably be included anyway but that would also be the case with std::copy.

I'm quite sure there are quite a few missing headers other places as well though. This is not a public header though so I don't consider it to be too much of an issue.