Test fgets_eagain fails on FreeBSD-amd64

According to the BSD Buildbot

util/fgets_eagain: 
  FAIL src/test/test_util.c:3952: assert(retptr OP_EQ buf): 0x0 vs 0x7fffffffe944
  [fgets_eagain FAILED]

This means that fgets returns a null pointer on partial lines instead of the buffer as expected.

Previously this test was passing but started failing with build #105. Looking at the changes to libc it looks like this is caused by revision 305413 (which was added earlier in the same month the test started failing).

I'm unsure whether FreeBSD is right and other libc implementations are wrong or the other way around.

changed milestone to %Tor: 0.3.1.x-final

added component::core tor/tor freebsd milestone::Tor: 0.3.1.x-final priority::medium resolution::fixed severity::normal status::closed tor-03-unspecified-201612 type::defect labels

Trac:
Keywords: N/A deleted, freebsd added
Milestone: N/A to Tor: 0.3.???

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

Trac:
Milestone: Tor: 0.3.??? to Tor: unspecified
Keywords: N/A deleted, tor-03-unspecified-201612 added

Closed #21416 (moved) as a duplicate.

I've added a patch for this in https://gitlab.com/ahf/tor/commits/bugs/20988

Trac:
Keywords: N/A deleted, review-group-16 added

Trac:
Milestone: Tor: unspecified to Tor: 0.3.1.x-final
Status: new to needs_review
Keywords: review-group-16 deleted, N/A added

Replying to ahf:

I've added a patch for this in https://gitlab.com/ahf/tor/commits/bugs/20988 OP here. The changes look good but i haven't tested them. The inline patch fixes some small grammar nitpicks on commit 49a4069d19877b24f00220fb41410445c804b463.

diff --git a/src/common/compat.c b/src/common/compat.c
index 771cc085d..753ad3f8f 100644
--- a/src/common/compat.c
+++ b/src/common/compat.c
@@ -3480,7 +3480,7 @@ tor_getpass(const char *prompt, char *output, size_t buflen)
  * Upon successful completion, this function returns a pointer to the string
  * <b>str</b>. If EOF occurs before any characters are read the function will
  * return NULL and the content of <b>str</b> is unchanged. Upon error, the
- * function return NULL and the caller must check for error using foef(3) and
+ * function returns NULL and the caller must check for error using foef(3) and
  * ferror(3).
  */
 char *
@@ -3490,10 +3490,10 @@ tor_fgets(char *str, int size, FILE *stream)
 
   ret = fgets(str, size, stream);
 
-  /* FreeBSD, OpenBSD, Linux (glibc), and Linux (musl) seems to disagree about
+  /* FreeBSD, OpenBSD, Linux (glibc), and Linux (musl) seem to disagree about
    * what to do in the given situation. We check if the stream has been flagged
-   * with an error-bit and retur NULL in that situation if errno is also set to
-   * EAGAIN.
+   * with an error-bit and return NULL in that situation if errno is also set
+   * to EAGAIN.
    */
   if (ferror(stream) && errno == EAGAIN)
     return NULL;

Looks good -- needs a changes file though? Also, is this tested?

Trac:
Status: needs_review to needs_revision

I've updated my branch with the following changes:

• Fixed typos reported by OP. Thanks!
• Added a changes file.
• We now clear the error flag on the file handler in the case where ferror(stream) && errno == EAGAIN to prevent the situation where the caller receives an error return value and retries because of errno being EAGAIN, but continues to have the error flag set.

I ran make check and tested it with "normal" browsing usage on OS X, HardenedBSD, and Linux.

Trac:
Cc: N/A to ahf
Status: needs_revision to needs_review

The clearerr() call is incorrect. Let me upload a revised version.

Trac:
Status: needs_review to needs_revision

Ignore the third bullet in comment 8: Instead of clearing the error flag of the file handle we now reset the errno variable before we call fgets(3).

Trac:
Status: needs_revision to needs_review

Merged this. Added #21654 (moved) for a more elegant solution.

Trac:
Resolution: N/A to fixed
Status: needs_review to closed

closed

mentioned in issue #21416 (moved)

mentioned in issue #21654 (moved)

mentioned in issue #23441 (moved)

moved to tpo/core/tor#20988 (closed)

mentioned in issue tpo/core/tor#21654 (closed)

mentioned in issue tpo/core/tor#23441 (closed)