On Tue, Sep 10, 2019 at 03:56:43PM +0200, Tim Düsterhus wrote: > Luca, > > Am 10.09.19 um 15:48 schrieb Schimweg, Luca: > > sorry, didn't know about these guidelines. I packed all changes into one > > commit in the patch file in the attachments. > > One more thing: Do not leave out the body of the commit message. See the > paragraph starting line 562 of the CONTRIBUTING file: > > https://github.com/haproxy/haproxy/blob/e058f7359f3822beb8552f77a6d439cb053edb3f/CONTRIBUTING#L562-L567 > > > I included your changes, you are right, the code looks much better now. > > I spotted a little typo: > > ((rnd[1] >> 16u) % 0xFFF) | 0x4000, // highest 4 bits indicate the uuid > version > > It should read `&` instead of `%` there. > > > I'm open for any feedback. > > I suggest to wait for Willy before sending an updated patch. I guess he > has some more feedback.
Thank to you both for the patch and the review. I agree with everything Tim said. I have oe other minor comment which is to add an argument check function to verify that the argument, if present, is 4, and default it to 4 when absent. Have a look at sample_conv_protobuf_check() and smp_check_concat() for example. Otherwise I'm fine with the patch. Cheers, Willy
