Hacker News new | past | comments | ask | show | jobs | submit login

I don't love the code style in terms of indentation, line length, alignment and bracket placing but IMO at least it doesn't look childish / reckless.

I think there could be bug in he_internal_send_auth_userpass when it copies the strings because when calculating the string lengths it uses the size of he_conn->username and he_conn->password which are "HE_CONFIG_TEXT_FIELD_LENGTH +1" whereas the sizes of the destination fields in he_msg_auth_t are "HE_CONFIG_TEXT_FIELD_LENGTH" so .

Take it with a grain of salt, I just took a very quick look mostly to see if I liked the coding style and it's far too early for my brain to be functional but it seemed that way to me. Other than that I didn't hate the code which is cool!

Thanks for opening it!




Disclaimer: I'm an employee of ExpressVPN and work on this codebase.

The specific reason for the size disparity is that we require he_conn->{username,password} to be null-terminated, whereas we do NOT require he_msg_auth->{username,password} to be null-terminated. I remember raising the same point and being convinced that we had a good reason for doing so, but I also haven't had enough coffee to remember what the good reason was!

Regardless of whether this is a "bug" or not I do think that the disparity points to something that unnecessarily causes confusion, will have a think about making it more consistent.

Regarding style -- I also ~hate if(x==0) without the space after the if but we committed to consistency instead of arguing over it via clang-format. To quote Rob Pike, "Gofmt's style is no one's favorite, yet gofmt is everyone's favorite."


Sorry about the late reply, I didn't realize there was a reply to my message. I reviewed it once again and can attest that what you say is correct, so that's cool.

Thanks for taking the time to reply!




Consider applying for YC's Fall 2025 batch! Applications are open till Aug 4

Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: