canonical / dqlite

Embeddable, replicated and fault-tolerant SQL engine.
https://dqlite.io
Other
3.83k stars 216 forks source link

client/protocol.c: Various improvements #496

Closed cole-miller closed 1 year ago

cole-miller commented 1 year ago

This is in preparation for the C client. One or two bug fixes (like making sure to return early in RESPONSE), some changes to integer types, refactoring the "strdup or OOM" pattern into a function, and adding support for custom connect functions.

Signed-off-by: Cole Miller cole.miller@canonical.com

freeekanayaka commented 1 year ago

Looks good.

In general, it'd would be nice to have a commit for each logical change, with a commit message that explains what was changed and why. It would make reviewing easier and also later on when reading code and trying to understand it.

cole-miller commented 1 year ago

Pushed a second attempt that's a bit cleaner (and I will try to exercise better commit discipline in the future).

MathieuBordere commented 1 year ago

If you could sort out the CI failures (you're probably on it).

cole-miller commented 1 year ago

Yep, sorry, there was a puzzling bug but I've fixed it now.

codecov[bot] commented 1 year ago

Codecov Report

Merging #496 (42b12dc) into master (a9a7d74) will decrease coverage by 0.15%. The diff coverage is 25.80%.

@@            Coverage Diff             @@
##           master     #496      +/-   ##
==========================================
- Coverage   58.97%   58.83%   -0.15%     
==========================================
  Files          33       33              
  Lines        5906     5900       -6     
  Branches     1769     1764       -5     
==========================================
- Hits         3483     3471      -12     
- Misses       1349     1361      +12     
+ Partials     1074     1068       -6     
Impacted Files Coverage Δ
src/server.c 51.39% <0.00%> (ø)
src/client/protocol.c 33.16% <25.00%> (-1.72%) :arrow_down:
src/gateway.c 43.59% <50.00%> (+0.01%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.