search

blindstore / blindstore

Official Blindstore repository
http://blindstore.github.io
MIT License
43 stars 7 forks source link

New cpp implementation #23

Closed andronat closed 9 years ago

andronat commented 10 years ago

Hello ppl,

I rewrote the code in C++ as we discussed with @bogdan-kulynych. I wrote the basic key generation for the client user. There is a make file in the directory, if you have gmp installed should work.

Please review as it been a long time since I wrote C++ :P

bogdan-kulynych commented 10 years ago

Not bad

andronat commented 10 years ago

ROFL

HarryCutts commented 9 years ago

Before I comment on the code itself, I'd prefer it if these commits were atomic, or at least left the code in a buildable state after each one. For example, if I were to check out the code at commit f48857, the C code wouldn't compile because KeyGenerator.h hasn't been created yet. I wouldn't even know how to compile it, because the makefile is created four commits further on.

It seems you've created these commits all at once, after you've done a good chunk of coding. It's much easier to create a series of working commits by committing as you code.

It also seems that commit 000eb77, as VDGHV isn't used by any of the other files yet.

I would suggest rebasing your branch to merge the first four commits, and excluding commit 000eb77 from the pull request. The dependencies between the files are such that they only make sense in one commit.

andronat commented 9 years ago

yes I agree. I will do the changes

HarryCutts commented 9 years ago

Whoa, sorry, I thought that GitHub would alert me when you changed the commits! Apologies for the lateness of this review.

I'll admit to not knowing much about C++, but I do know that in C++11 raw pointers are unnecessary, and can cause security problems. A few of the uses of pointers here (like *sk on line 9 of KeyGenerator.cpp) could be replaced with references, in fact (since the parameter is never changed during function execution). Could the others not be replaced with smart pointers, or maybe even return by value (depending on the exact size of an mpz_class)?

andronat commented 9 years ago

Hi @Fodaro, is there any possibility, you to make a pull request on my repo so I can accept your changes? I will not have time until the mid of next week :/

HarryCutts commented 9 years ago

@andronat: I'm afraid I won't have any time before then either, and I'm not really confident enough with C++ to do it.