RE: boost::optional<> in librados

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



I think 3 is the right way. Also, bufferlist sha1 code can be enhanced as Piotr said.
I will fix it.

Myoungwon

-----Original Message-----
From: Piotr Dałek <piotr.dalek@xxxxxxxxxxxx> 
Sent: Wednesday, September 12, 2018 8:10 PM
To: kefu chai <tchaikov@xxxxxxxxx>; 오명원님/블록체인Tech.Lab <omwmw@xxxxxx>; The Esoteric Order of the Squid Cybernetic <ceph-devel@xxxxxxxxxxxxxxx>
Subject: Re: boost::optional<> in librados

On 18-09-12 12:59 PM, kefu chai wrote:
> hi guys,
> 
> i just found that librados now depends on boost because of the newly 
> introduced boost::optional<sha1_digest_t> ceph::buffer::list::sha1() 
> method in [0]. and it turns out to be an issue when compiling an 
> librados application [1] in a clean-room environment.
> 
> so we have two options:
> 
> 1. claim that librados's C++ binding is now moving to C++17. and use 
> std::optional<> instead of boost<> in librados.hpp.
>    - pros: libboost won't be a dependency of librados-dev / librados-devel.
>    - cons: our users are forced to use C++17 for compiling their C++ 
> application with librados since nautilus 2. make libboost-dev / 
> libboot-devel a dependency of librados-dev. et voilà.
>    - pros: users can choose to stay with C++11.
>    - cons: another dependency at build time. user will need to install 
> libboost-dev or libboost-devel in their CI script or building host.
> 3. change the sha1() method to a more complicated form, for example, 
> it could return an std::pair<bool, sha1_digest_t>.

3, or just return sha1_digest_t* which can be nullptr.

Plus, unless I'm missing something, that bufferlist sha1 code is inefficient, as it copies entire bufferlist into single bufferptr and calculates sha1 off that ptr. It could call sha1_gen.Update for each bufferptr separately without any copies involved.

--
Piotr Dałek
piotr.dalek@xxxxxxxxxxxx
https://www.ovhcloud.com




[Index of Archives]     [CEPH Users]     [Ceph Large]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux