Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ticket26223 #219

Merged
merged 4 commits into from Jul 11, 2018
Merged

Ticket26223 #219

merged 4 commits into from Jul 11, 2018

Conversation

Reviewers

@teor2345 teor2345

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@nmathewson @coveralls @teor2345 @torproject-pusher
@nmathewson
Copy link
Contributor

@nmathewson nmathewson commented Jul 10, 2018

No description provided.

nmathewson added 4 commits Jul 10, 2018
@nmathewson
Add the compatibility definition for getdelim.c from netbsd.
1604c0f 
We shouldn't actually need this code nearly anywhere we build:
getdelim is POSIX, and mingw provides it.
@nmathewson
Integrate getdelim() and getline() support into Tor.
b04d719
@nmathewson
Refactor dirserv_read_measured_bandwidths to have a single exit point
6574d4b
@nmathewson
Use tor_getline() in dirserv.c to remove its upper line limit.
951d59d 
Closes ticket 26223.
@coveralls
Copy link

@coveralls coveralls commented Jul 10, 2018

Coverage Status

Coverage decreased (-0.02%) to 59.485% when pulling 951d59d on nmathewson:ticket26223 into 3145e46 on torproject:master.

teor2345
teor2345 reviewed Jul 10, 2018
View changes
Copy link
Contributor

@teor2345 teor2345 left a comment

Looks great!
We could add a few extra comments if you like.

configure.ac
getifaddrs \
getline \
Copy link
Contributor

@teor2345 teor2345 Jul 10, 2018

Why are these lines misaligned?
Is it a tabs and spaces thing?

Copy link
Contributor Author

@nmathewson nmathewson Jul 11, 2018

Yup; added a commit to retabify configure.ac's lists.

src/feature/dircache/dirserv.c

err:
if (line)
raw_free(line);
Copy link
Contributor

@teor2345 teor2345 Jul 10, 2018

Let's add a comment saying that we use raw_free() because getdelim() uses raw_malloc().

Copy link
Contributor Author

@nmathewson nmathewson Jul 11, 2018

ok

src/lib/fs/files.h
@@ -103,4 +103,38 @@ char *read_file_to_str_until_eof(int fd, size_t max_bytes_to_read,
size_t *sz_out)
ATTR_MALLOC;

#if !defined(HAVE_GETDELIM) || defined(TOR_UNIT_TESTS)
ssize_t compat_getdelim_(char **lineptr, size_t *n, int delim, FILE *stream);
Copy link
Contributor

@teor2345 teor2345 Jul 10, 2018

There's no function comment for getdelim() - do we expect developers to look it up in the relevant man page?

Copy link
Contributor Author

@nmathewson nmathewson Jul 11, 2018

I think that's best -- since this is a POSIX standard, if our function differs from the manpage, one of them is wrong.

@torproject-pusher torproject-pusher merged commit 951d59d into torproject:master Jul 11, 2018
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment