diff mbox series

[5/6] Remove miscellaneous debris from libio.

Message ID 20180211163558.14124-6-zackw@panix.com
State New
Headers show
Series Mostly mechanical libio decrufting. | expand

Commit Message

Zack Weinberg Feb. 11, 2018, 4:35 p.m. UTC
This patch eliminates a number of #if 0 and #ifdef TODO blocks, macros
that are never used, macros that provide portability to substrates that
lack basic things like EINVAL and off_t, and other such debris.

I preserved IO_DEBUG and CHECK_FILE, even though as far as I can tell
IO_DEBUG is never defined and therefore CHECK_FILE never does
anything, because it seems like we might actually want to turn it _on_.

	* libio/libio.h (_IO_pos_BAD, _IO_pos_0, _IO_pos_adjust):
	Define here, unconditionally.
	* libio/iolibio.h (_IO_pos_BAD): Don't define here.
	* libio/libioP.h: Remove #if 0 blocks.
	(_IO_pos_BAD, _IO_pos_0, _IO_pos_adjust): Don't define here.
	(_IO_va_start, COERCE_FILE, MAYBE_SET_EINVAL): Don't define.
	(CHECK_FILE): Don't use MAYBE_SET_EINVAL or COERCE_FILE.  Fix style.

	* libio/clearerr.c, libio/fputc.c, libio/getchar.c:
	Assume weak_alias is always defined.

	* libio/fileops.c, libio/genops.c, libio/oldfileops.c
	* libio/oldpclose.c, libio/pclose.c, libio/wfileops.c:
	Remove #if 0 and #ifdef TODO blocks.
	Assume text_set_element is always defined.

	* libio/iofdopen.c, libio/iogetdelim.c, libio/oldiofdopen.c
	Use __set_errno (EINVAL) instead of MAYBE_SET_EINVAL.
	* libio/tst-mmap-eofsync.c: Make #if 1 block unconditional.
---
 libio/clearerr.c         |   2 +-
 libio/fileops.c          |  18 --------
 libio/fputc.c            |   2 +-
 libio/genops.c           | 116 +----------------------------------------------
 libio/getchar.c          |   2 +-
 libio/iofdopen.c         |   4 +-
 libio/iogetdelim.c       |   2 +-
 libio/iolibio.h          |   3 --
 libio/libio.h            |   8 ++++
 libio/libioP.h           |  49 ++++----------------
 libio/oldfileops.c       |   9 ----
 libio/oldiofdopen.c      |   2 +-
 libio/oldpclose.c        |   8 +---
 libio/pclose.c           |   8 +---
 libio/tst-mmap-eofsync.c |   2 -
 libio/wfileops.c         |   5 --
 libio/wgenops.c          |  34 --------------
 stdio-common/vfprintf.c  |   2 +-
 stdio-common/vfscanf.c   |   2 +-
 19 files changed, 32 insertions(+), 246 deletions(-)

Comments

Andreas Schwab Feb. 11, 2018, 6:07 p.m. UTC | #1
On Feb 11 2018, Zack Weinberg <zackw@panix.com> wrote:

> diff --git a/libio/fileops.c b/libio/fileops.c
> index fb39bec63cd..618c0d5c311 100644
> --- a/libio/fileops.c
> +++ b/libio/fileops.c
> @@ -468,11 +468,6 @@ int
>  _IO_new_file_underflow (FILE *fp)
>  {
>    ssize_t count;
> -#if 0
> -  /* SysV does not make this test; take it out for compatibility */
> -  if (fp->_flags & _IO_EOF_SEEN)
> -    return (EOF);
> -#endif

AFAICS, this is an area where we are not conforming to the C standard
(the EOF indicator is supposed to be sticking, see fgetc, and only
clearerr can fix that).

Andreas.
Zack Weinberg Feb. 11, 2018, 6:53 p.m. UTC | #2
On Sun, Feb 11, 2018 at 1:07 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> On Feb 11 2018, Zack Weinberg <zackw@panix.com> wrote:
>
>> diff --git a/libio/fileops.c b/libio/fileops.c
>> index fb39bec63cd..618c0d5c311 100644
>> --- a/libio/fileops.c
>> +++ b/libio/fileops.c
>> @@ -468,11 +468,6 @@ int
>>  _IO_new_file_underflow (FILE *fp)
>>  {
>>    ssize_t count;
>> -#if 0
>> -  /* SysV does not make this test; take it out for compatibility */
>> -  if (fp->_flags & _IO_EOF_SEEN)
>> -    return (EOF);
>> -#endif
>
> AFAICS, this is an area where we are not conforming to the C standard
> (the EOF indicator is supposed to be sticking, see fgetc, and only
> clearerr can fix that).

Yes, I think you're right.  I would generally be in favor of changing
to conform; the BSD lineage was always conformant and I think
compatibility with those is more important than compatibility with
legacy SysV nowadays.  And we're already contemplating a bunch of
stdio-related changes in this release cycle, so now is a good time.

I don't want to make any behavioral changes in this patch series,
though, and if I changed it I'd need to write a test case and that
looks nontrivial right now, so what I'll do is leave those #if 0
blocks in place and maybe come back to it in a separate discussion.
Sound good?

zw
Andreas Schwab Feb. 11, 2018, 7:07 p.m. UTC | #3
On Feb 11 2018, Zack Weinberg <zackw@panix.com> wrote:

> I don't want to make any behavioral changes in this patch series,
> though, and if I changed it I'd need to write a test case and that
> looks nontrivial right now, so what I'll do is leave those #if 0
> blocks in place and maybe come back to it in a separate discussion.
> Sound good?

Yes, that is what I had in mind.

Andreas.
Joseph Myers Feb. 12, 2018, 4:37 p.m. UTC | #4
On Sun, 11 Feb 2018, Andreas Schwab wrote:

> AFAICS, this is an area where we are not conforming to the C standard
> (the EOF indicator is supposed to be sticking, see fgetc, and only
> clearerr can fix that).

This is bug 1190.
Zack Weinberg Feb. 12, 2018, 4:39 p.m. UTC | #5
On Mon, Feb 12, 2018 at 11:37 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Sun, 11 Feb 2018, Andreas Schwab wrote:
>
>> AFAICS, this is an area where we are not conforming to the C standard
>> (the EOF indicator is supposed to be sticking, see fgetc, and only
>> clearerr can fix that).
>
> This is bug 1190.

Funny you should mention that ...

zw
Zack Weinberg Feb. 13, 2018, 3:17 p.m. UTC | #6
On Sun, Feb 11, 2018 at 11:35 AM, Zack Weinberg <zackw@panix.com> wrote:
> This patch eliminates a number of #if 0 and #ifdef TODO blocks, macros
> that are never used, macros that provide portability to substrates that
> lack basic things like EINVAL and off_t, and other such debris.

Apart from Andreas' comment about the lack of sticky EOF, this patch
doesn't appear to have been reviewed; all the other patches in the
series have been reviewed, so would anyone like to step up and look at
this one?

zw
Florian Weimer Feb. 19, 2018, 5:01 p.m. UTC | #7
On 02/11/2018 05:35 PM, Zack Weinberg wrote:
> -#if 0 /* Work in progress */
> -/* Seems not to be needed.  */
> -#if 0
> -void
> -_IO_set_column (FILE *fp, int c)

I'm glad that's gone.  8-P

The rest of the patch is okay, except I'm not sure if moving around this 
comment is the right thing:

> +/* Does not actually test that stream was created by popen(). Instead,
> +   it depends on the filebuf::sys_close() virtual to Do The Right Thing. */

It doesn't look very informative and completely up-to-date to me.  But I 
don't think it's a good use of our time to dwell on this.

Do fclose and pclose have to have different addresses?  We could make 
them aliases.


Thanks,
Florian
Florian Weimer Feb. 19, 2018, 6:32 p.m. UTC | #8
On 02/11/2018 07:07 PM, Andreas Schwab wrote:
> AFAICS, this is an area where we are not conforming to the C standard
> (the EOF indicator is supposed to be sticking, see fgetc, and only
> clearerr can fix that).

But is this actually observable without violating any of the stream 
synchronization requirements, or relying on implementation-defined 
behavior of file descriptors?

I'm not convinced that the current behavior is actually non-compliant.

Thanks,
Florian
Zack Weinberg Feb. 19, 2018, 7:12 p.m. UTC | #9
On Mon, Feb 19, 2018 at 1:32 PM, Florian Weimer <fweimer@redhat.com> wrote:
> On 02/11/2018 07:07 PM, Andreas Schwab wrote:
>>
>> AFAICS, this is an area where we are not conforming to the C standard
>> (the EOF indicator is supposed to be sticking, see fgetc, and only
>> clearerr can fix that).
>
>
> But is this actually observable without violating any of the stream
> synchronization requirements, or relying on implementation-defined behavior
> of file descriptors?

I think so, see the test case in
https://sourceware.org/ml/libc-alpha/2018-02/msg00420.html .

zw
Florian Weimer Feb. 19, 2018, 7:29 p.m. UTC | #10
On 02/19/2018 08:12 PM, Zack Weinberg wrote:
> On Mon, Feb 19, 2018 at 1:32 PM, Florian Weimer <fweimer@redhat.com> wrote:
>> On 02/11/2018 07:07 PM, Andreas Schwab wrote:
>>>
>>> AFAICS, this is an area where we are not conforming to the C standard
>>> (the EOF indicator is supposed to be sticking, see fgetc, and only
>>> clearerr can fix that).
>>
>>
>> But is this actually observable without violating any of the stream
>> synchronization requirements, or relying on implementation-defined behavior
>> of file descriptors?
> 
> I think so, see the test case in
> https://sourceware.org/ml/libc-alpha/2018-02/msg00420.html .

Do you mean the on in this patch:

   https://sourceware.org/ml/libc-alpha/2018-02/msg00416.html

I believe the test case is invalid because interleaving file access in 
this way requires that fseek is used before reading any data, and this 
will clear the EOF indicator.

Thanks,
Florian
Andreas Schwab Feb. 19, 2018, 8:10 p.m. UTC | #11
On Feb 19 2018, Florian Weimer <fweimer@redhat.com> wrote:

> But is this actually observable without violating any of the stream
> synchronization requirements, or relying on implementation-defined
> behavior of file descriptors?

Just point the stream to a tty and type an EOF character.

Andreas.
Florian Weimer Feb. 20, 2018, 8:18 a.m. UTC | #12
On 02/19/2018 09:10 PM, Andreas Schwab wrote:
> On Feb 19 2018, Florian Weimer <fweimer@redhat.com> wrote:
> 
>> But is this actually observable without violating any of the stream
>> synchronization requirements, or relying on implementation-defined
>> behavior of file descriptors?
> 
> Just point the stream to a tty and type an EOF character.

Shouldn't fgetc block and wait for more characters in this case?

Thanks,
Florian
Andreas Schwab Feb. 20, 2018, 6:16 p.m. UTC | #13
On Feb 20 2018, Florian Weimer <fweimer@redhat.com> wrote:

> On 02/19/2018 09:10 PM, Andreas Schwab wrote:
>> On Feb 19 2018, Florian Weimer <fweimer@redhat.com> wrote:
>>
>>> But is this actually observable without violating any of the stream
>>> synchronization requirements, or relying on implementation-defined
>>> behavior of file descriptors?
>>
>> Just point the stream to a tty and type an EOF character.
>
> Shouldn't fgetc block and wait for more characters in this case?

No, it should get EOF immediately, since that is supposed to be sticky.

Andreas.
Zack Weinberg Feb. 21, 2018, 7:44 p.m. UTC | #14
On Mon, Feb 19, 2018 at 12:01 PM, Florian Weimer <fweimer@redhat.com> wrote:
> On 02/11/2018 05:35 PM, Zack Weinberg wrote:
>> +/* Does not actually test that stream was created by popen(). Instead,
>> +   it depends on the filebuf::sys_close() virtual to Do The Right Thing.
>> */
>
> It doesn't look very informative and completely up-to-date to me.  But I
> don't think it's a good use of our time to dwell on this.

I think it's useful to remark that POSIX doesn't require us to check
that a stream passed to pclose was created by popen (or that a stream
passed to fclose was created by fopen).  I changed the comment to say
that, and to refer to _IO_SYSCLOSE instead of "the filebuf::sys_close
virtual".

> Do fclose and pclose have to have different addresses?  We could make them
> aliases.

I don't know how to find out the answer to this question.  We do make
fputc and putc have the same address...

zw
Joseph Myers Feb. 21, 2018, 10:39 p.m. UTC | #15
On Wed, 21 Feb 2018, Zack Weinberg wrote:

> > Do fclose and pclose have to have different addresses?  We could make them
> > aliases.
> 
> I don't know how to find out the answer to this question.  We do make
> fputc and putc have the same address...

The question of explicitly allowing aliasing between different standard 
library functions with the same type (I think it's clearly allowed between 
functions with different types, simply because of the limitations of what 
you can do with a function pointer converted to a different type) is on my 
list of issues to raise formally with WG14 papers for C2x.

The footnote "There is no linkage between different identifiers." is not 
normative and can reasonably be interpreted as meaning "there is no way in 
standard C to declare there to be a linkage between different identifiers" 
without saying anything about how such linkage might be created for 
standard library functions that aren't in standard C.
diff mbox series

Patch

diff --git a/libio/clearerr.c b/libio/clearerr.c
index 5d29fc500a0..77ac552fac1 100644
--- a/libio/clearerr.c
+++ b/libio/clearerr.c
@@ -27,6 +27,6 @@  clearerr (FILE *fp)
   _IO_funlockfile (fp);
 }
 
-#if defined weak_alias && !defined _IO_MTSAFE_IO
+#ifndef _IO_MTSAFE_IO
 weak_alias (clearerr, clearerr_unlocked)
 #endif
diff --git a/libio/fileops.c b/libio/fileops.c
index fb39bec63cd..618c0d5c311 100644
--- a/libio/fileops.c
+++ b/libio/fileops.c
@@ -468,11 +468,6 @@  int
 _IO_new_file_underflow (FILE *fp)
 {
   ssize_t count;
-#if 0
-  /* SysV does not make this test; take it out for compatibility */
-  if (fp->_flags & _IO_EOF_SEEN)
-    return (EOF);
-#endif
 
   if (fp->_flags & _IO_NO_READS)
     {
@@ -494,13 +489,9 @@  _IO_new_file_underflow (FILE *fp)
       _IO_doallocbuf (fp);
     }
 
-  /* Flush all line buffered files before reading. */
   /* FIXME This can/should be moved to genops ?? */
   if (fp->_flags & (_IO_LINE_BUF|_IO_UNBUFFERED))
     {
-#if 0
-      _IO_flush_all_linebuffered ();
-#else
       /* We used to flush all line-buffered stream.  This really isn't
 	 required by any standard.  My recollection is that
 	 traditional Unix systems did this for stdout.  stderr better
@@ -513,7 +504,6 @@  _IO_new_file_underflow (FILE *fp)
 	_IO_OVERFLOW (_IO_stdout, EOF);
 
       _IO_release_lock (_IO_stdout);
-#endif
     }
 
   _IO_switch_to_get_mode (fp);
@@ -813,10 +803,6 @@  _IO_new_file_sync (FILE *fp)
   delta = fp->_IO_read_ptr - fp->_IO_read_end;
   if (delta != 0)
     {
-#ifdef TODO
-      if (_IO_in_backup (fp))
-	delta -= eGptr () - Gbase ();
-#endif
       off64_t new_pos = _IO_SYSSEEK (fp, delta, 1);
       if (new_pos != (off64_t) EOF)
 	fp->_IO_read_end = fp->_IO_read_ptr;
@@ -838,10 +824,6 @@  _IO_file_sync_mmap (FILE *fp)
 {
   if (fp->_IO_read_ptr != fp->_IO_read_end)
     {
-#ifdef TODO
-      if (_IO_in_backup (fp))
-	delta -= eGptr () - Gbase ();
-#endif
       if (__lseek64 (fp->_fileno, fp->_IO_read_ptr - fp->_IO_buf_base,
 		     SEEK_SET)
 	  != fp->_IO_read_ptr - fp->_IO_buf_base)
diff --git a/libio/fputc.c b/libio/fputc.c
index 13b3b84a769..1e743eb74c9 100644
--- a/libio/fputc.c
+++ b/libio/fputc.c
@@ -40,7 +40,7 @@  fputc (int c, FILE *fp)
   return result;
 }
 
-#if defined weak_alias && !defined _IO_MTSAFE_IO
+#ifndef _IO_MTSAFE_IO
 #undef fputc_unlocked
 weak_alias (fputc, fputc_unlocked)
 #endif
diff --git a/libio/genops.c b/libio/genops.c
index 2b820c86d20..2fec221b997 100644
--- a/libio/genops.c
+++ b/libio/genops.c
@@ -194,24 +194,6 @@  _IO_free_backup_area (FILE *fp)
 }
 libc_hidden_def (_IO_free_backup_area)
 
-#if 0
-int
-_IO_switch_to_put_mode (FILE *fp)
-{
-  fp->_IO_write_base = fp->_IO_read_ptr;
-  fp->_IO_write_ptr = fp->_IO_read_ptr;
-  /* Following is wrong if line- or un-buffered? */
-  fp->_IO_write_end = (fp->_flags & _IO_IN_BACKUP
-		       ? fp->_IO_read_end : fp->_IO_buf_end);
-
-  fp->_IO_read_ptr = fp->_IO_read_end;
-  fp->_IO_read_base = fp->_IO_read_end;
-
-  fp->_flags |= _IO_CURRENTLY_PUTTING;
-  return 0;
-}
-#endif
-
 int
 __overflow (FILE *f, int ch)
 {
@@ -465,15 +447,6 @@  _IO_default_xsgetn (FILE *fp, void *data, size_t n)
 }
 libc_hidden_def (_IO_default_xsgetn)
 
-#if 0
-/* Seems not to be needed. --drepper */
-int
-_IO_sync (FILE *fp)
-{
-  return 0;
-}
-#endif
-
 FILE *
 _IO_default_setbuf (FILE *fp, char *p, ssize_t len)
 {
@@ -697,28 +670,6 @@  _IO_sungetc (FILE *fp)
   return result;
 }
 
-#if 0 /* Work in progress */
-/* Seems not to be needed.  */
-#if 0
-void
-_IO_set_column (FILE *fp, int c)
-{
-  if (c == -1)
-    fp->_column = -1;
-  else
-    fp->_column = c - (fp->_IO_write_ptr - fp->_IO_write_base);
-}
-#else
-int
-_IO_set_column (FILE *fp, int i)
-{
-  fp->_cur_column = i + 1;
-  return 0;
-}
-#endif
-#endif
-
-
 unsigned
 _IO_adjust_column (unsigned start, const char *line, int count)
 {
@@ -730,20 +681,6 @@  _IO_adjust_column (unsigned start, const char *line, int count)
 }
 libc_hidden_def (_IO_adjust_column)
 
-#if 0
-/* Seems not to be needed. --drepper */
-int
-_IO_get_column (FILE *fp)
-{
-  if (fp->_cur_column)
-    return _IO_adjust_column (fp->_cur_column - 1,
-			      fp->_IO_write_base,
-			      fp->_IO_write_ptr - fp->_IO_write_base);
-  return -1;
-}
-#endif
-
-
 int
 _IO_flush_all_lockp (int do_lock)
 {
@@ -964,10 +901,8 @@  _IO_remove_marker (struct _IO_marker *marker)
 	  return;
 	}
     }
-#if 0
-    if _sbuf has a backup area that is no longer needed, should we delete
-    it now, or wait until the next underflow?
-#endif
+  /* FIXME: if _sbuf has a backup area that is no longer needed,
+     should we delete it now, or wait until the next underflow? */
 }
 
 #define BAD_DELTA EOF
@@ -1018,20 +953,6 @@  _IO_unsave_markers (FILE *fp)
   struct _IO_marker *mark = fp->_markers;
   if (mark)
     {
-#ifdef TODO
-      streampos offset = seekoff (0, ios::cur, ios::in);
-      if (offset != EOF)
-	{
-	  offset += eGptr () - Gbase ();
-	  for ( ; mark != NULL; mark = mark->_next)
-	    mark->set_streampos (mark->_pos + offset);
-	}
-    else
-      {
-	for ( ; mark != NULL; mark = mark->_next)
-	  mark->set_streampos (EOF);
-      }
-#endif
       fp->_markers = 0;
     }
 
@@ -1040,19 +961,6 @@  _IO_unsave_markers (FILE *fp)
 }
 libc_hidden_def (_IO_unsave_markers)
 
-#if 0
-/* Seems not to be needed. --drepper */
-int
-_IO_nobackup_pbackfail (FILE *fp, int c)
-{
-  if (fp->_IO_read_ptr > fp->_IO_read_base)
-	fp->_IO_read_ptr--;
-  if (c != EOF && *fp->_IO_read_ptr != c)
-      *fp->_IO_read_ptr = c;
-  return (unsigned char) c;
-}
-#endif
-
 int
 _IO_default_pbackfail (FILE *fp, int c)
 {
@@ -1200,24 +1108,4 @@  _IO_list_resetlock (void)
 }
 libc_hidden_def (_IO_list_resetlock)
 
-
-#ifdef TODO
-#if defined(linux)
-#define IO_CLEANUP ;
-#endif
-
-#ifdef IO_CLEANUP
-  IO_CLEANUP
-#else
-struct __io_defs {
-    __io_defs() { }
-    ~__io_defs() { _IO_cleanup (); }
-};
-__io_defs io_defs__;
-#endif
-
-#endif /* TODO */
-
-#ifdef text_set_element
 text_set_element(__libc_atexit, _IO_cleanup);
-#endif
diff --git a/libio/getchar.c b/libio/getchar.c
index 8f8555f8c1e..0e3f4153c89 100644
--- a/libio/getchar.c
+++ b/libio/getchar.c
@@ -41,7 +41,7 @@  getchar (void)
   return result;
 }
 
-#if defined weak_alias && !defined _IO_MTSAFE_IO
+#ifndef _IO_MTSAFE_IO
 #undef getchar_unlocked
 weak_alias (getchar, getchar_unlocked)
 #endif
diff --git a/libio/iofdopen.c b/libio/iofdopen.c
index 21a53e30f6c..1f20eacb256 100644
--- a/libio/iofdopen.c
+++ b/libio/iofdopen.c
@@ -62,7 +62,7 @@  _IO_new_fdopen (int fd, const char *mode)
       read_write = _IO_NO_READS|_IO_IS_APPENDING;
       break;
     default:
-      MAYBE_SET_EINVAL;
+      __set_errno (EINVAL);
       return NULL;
   }
   for (i = 1; i < 5; ++i)
@@ -92,7 +92,7 @@  _IO_new_fdopen (int fd, const char *mode)
   if (((fd_flags & O_ACCMODE) == O_RDONLY && !(read_write & _IO_NO_WRITES))
       || ((fd_flags & O_ACCMODE) == O_WRONLY && !(read_write & _IO_NO_READS)))
     {
-      MAYBE_SET_EINVAL;
+      __set_errno (EINVAL);
       return NULL;
     }
 
diff --git a/libio/iogetdelim.c b/libio/iogetdelim.c
index f5b0991be8c..4f3ce5ed62c 100644
--- a/libio/iogetdelim.c
+++ b/libio/iogetdelim.c
@@ -45,7 +45,7 @@  _IO_getdelim (char **lineptr, size_t *n, int delimiter, FILE *fp)
 
   if (lineptr == NULL || n == NULL)
     {
-      MAYBE_SET_EINVAL;
+      __set_errno (EINVAL);
       return -1;
     }
   CHECK_FILE (fp, -1);
diff --git a/libio/iolibio.h b/libio/iolibio.h
index ceaa33a44bb..69e1c0e699c 100644
--- a/libio/iolibio.h
+++ b/libio/iolibio.h
@@ -59,9 +59,6 @@  struct obstack;
 extern int _IO_obstack_vprintf (struct obstack *, const char *, __gnuc_va_list)
        __THROW;
 extern int _IO_obstack_printf (struct obstack *, const char *, ...) __THROW;
-#ifndef _IO_pos_BAD
-#define _IO_pos_BAD ((off64_t)(-1))
-#endif
 #define _IO_clearerr(FP) ((FP)->_flags &= ~(_IO_ERR_SEEN|_IO_EOF_SEEN))
 #define _IO_fseek(__fp, __offset, __whence) \
   (_IO_seekoff_unlocked (__fp, __offset, __whence, _IOS_INPUT|_IOS_OUTPUT) \
diff --git a/libio/libio.h b/libio/libio.h
index d1516185667..1371161e025 100644
--- a/libio/libio.h
+++ b/libio/libio.h
@@ -124,6 +124,14 @@  typedef union
 #define _IO_DONT_CLOSE 0100000
 #define _IO_BOOLALPHA 0200000
 
+/* _IO_pos_BAD is an off64_t value indicating error, unknown, or EOF. */
+#define _IO_pos_BAD ((off64_t) -1)
+
+/* _IO_pos_adjust adjust an off64_t by some number of bytes. */
+#define _IO_pos_adjust(pos, delta) ((pos) += (delta))
+
+/* _IO_pos_0 is an off64_t value indicating beginning of file. */
+#define _IO_pos_0 ((off64_t) 0)
 
 struct _IO_jump_t;
 
diff --git a/libio/libioP.h b/libio/libioP.h
index ee23296ddba..8afe7032e3f 100644
--- a/libio/libioP.h
+++ b/libio/libioP.h
@@ -309,10 +309,6 @@  struct _IO_jump_t
     JUMP_FIELD(_IO_stat_t, __stat);
     JUMP_FIELD(_IO_showmanyc_t, __showmanyc);
     JUMP_FIELD(_IO_imbue_t, __imbue);
-#if 0
-    get_column;
-    set_column;
-#endif
 };
 
 /* We always allocate an extra word following an _IO_FILE.
@@ -710,19 +706,6 @@  extern off64_t _IO_seekpos_unlocked (FILE *, off64_t, int)
 
 extern int _IO_vscanf (const char *, va_list) __THROW;
 
-/* _IO_pos_BAD is an off64_t value indicating error, unknown, or EOF. */
-#ifndef _IO_pos_BAD
-# define _IO_pos_BAD ((off64_t) -1)
-#endif
-/* _IO_pos_adjust adjust an off64_t by some number of bytes. */
-#ifndef _IO_pos_adjust
-# define _IO_pos_adjust(pos, delta) ((pos) += (delta))
-#endif
-/* _IO_pos_0 is an off64_t value indicating beginning of file. */
-#ifndef _IO_pos_0
-# define _IO_pos_0 ((off64_t) 0)
-#endif
-
 #ifdef _IO_MTSAFE_IO
 /* check following! */
 # ifdef _IO_USE_OLD_IO_FILE
@@ -752,33 +735,19 @@  extern int _IO_vscanf (const char *, va_list) __THROW;
 # endif
 #endif
 
-#define _IO_va_start(args, last) va_start(args, last)
-
 extern struct _IO_fake_stdiobuf _IO_stdin_buf, _IO_stdout_buf, _IO_stderr_buf;
 
-#if 1
-# define COERCE_FILE(FILE) /* Nothing */
-#else
-/* This is part of the kludge for binary compatibility with old stdio. */
-# define COERCE_FILE(FILE) \
-  (((FILE)->_flags & _IO_MAGIC_MASK) == _OLD_MAGIC_MASK \
-    && (FILE) = *(FILE**)&((int*)fp)[1])
-#endif
-
-#ifdef EINVAL
-# define MAYBE_SET_EINVAL __set_errno (EINVAL)
-#else
-# define MAYBE_SET_EINVAL /* nothing */
-#endif
-
 #ifdef IO_DEBUG
-# define CHECK_FILE(FILE, RET) \
-	if ((FILE) == NULL) { MAYBE_SET_EINVAL; return RET; } \
-	else { COERCE_FILE(FILE); \
-	       if (((FILE)->_flags & _IO_MAGIC_MASK) != _IO_MAGIC) \
-	  { MAYBE_SET_EINVAL; return RET; }}
+# define CHECK_FILE(FILE, RET) do {			\
+    if ((FILE) == NULL ||				\
+	((FILE)->_flags & _IO_MAGIC_MASK) != _IO_MAGIC) \
+      {							\
+	__set_errno (EINVAL);				\
+	return RET;					\
+      }							\
+  } while (0)
 #else
-# define CHECK_FILE(FILE, RET) COERCE_FILE (FILE)
+# define CHECK_FILE(FILE, RET) do { } while (0)
 #endif
 
 static inline void
diff --git a/libio/oldfileops.c b/libio/oldfileops.c
index 5a0f246411c..b53dcacbbc7 100644
--- a/libio/oldfileops.c
+++ b/libio/oldfileops.c
@@ -294,11 +294,6 @@  attribute_compat_text_section
 _IO_old_file_underflow (FILE *fp)
 {
   ssize_t count;
-#if 0
-  /* SysV does not make this test; take it out for compatibility */
-  if (fp->_flags & _IO_EOF_SEEN)
-    return (EOF);
-#endif
 
   if (fp->_flags & _IO_NO_READS)
     {
@@ -516,10 +511,6 @@  _IO_old_file_seekoff (FILE *fp, off64_t offset, int dir, int mode)
 			      + (fp->_IO_read_end - fp->_IO_read_base));
       if (rel_offset >= 0)
 	{
-#if 0
-	  if (_IO_in_backup (fp))
-	    _IO_switch_to_main_get_area (fp);
-#endif
 	  if (rel_offset <= fp->_IO_read_end - fp->_IO_read_base)
 	    {
 	      _IO_setg (fp, fp->_IO_buf_base, fp->_IO_buf_base + rel_offset,
diff --git a/libio/oldiofdopen.c b/libio/oldiofdopen.c
index 17930c2786f..cec6642fc40 100644
--- a/libio/oldiofdopen.c
+++ b/libio/oldiofdopen.c
@@ -60,7 +60,7 @@  _IO_old_fdopen (int fd, const char *mode)
       read_write = _IO_NO_READS|_IO_IS_APPENDING;
       break;
     default:
-      MAYBE_SET_EINVAL;
+      __set_errno (EINVAL);
       return NULL;
   }
   if (mode[0] == '+' || (mode[0] == 'b' && mode[1] == '+'))
diff --git a/libio/oldpclose.c b/libio/oldpclose.c
index f02a05fc376..d2975206a3d 100644
--- a/libio/oldpclose.c
+++ b/libio/oldpclose.c
@@ -32,16 +32,12 @@ 
 #include "stdio.h"
 #include <errno.h>
 
+/* Does not actually test that stream was created by popen(). Instead,
+   it depends on the filebuf::sys_close() virtual to Do The Right Thing. */
 int
 attribute_compat_text_section
 __old_pclose (FILE *fp)
 {
-#if 0
-  /* Does not actually test that stream was created by popen(). Instead,
-     it depends on the filebuf::sys_close() virtual to Do The Right Thing. */
-  if (fp is not a proc_file)
-    return -1;
-#endif
   return _IO_old_fclose (fp);
 }
 
diff --git a/libio/pclose.c b/libio/pclose.c
index 74851b55ba3..052efaa31ed 100644
--- a/libio/pclose.c
+++ b/libio/pclose.c
@@ -29,15 +29,11 @@ 
 #include <errno.h>
 #include <shlib-compat.h>
 
+/* Does not actually test that stream was created by popen(). Instead,
+   it depends on the filebuf::sys_close() virtual to Do The Right Thing. */
 int
 __new_pclose (FILE *fp)
 {
-#if 0
-  /* Does not actually test that stream was created by popen(). Instead,
-     it depends on the filebuf::sys_close() virtual to Do The Right Thing. */
-  if (fp is not a proc_file)
-    return -1;
-#endif
   return _IO_new_fclose (fp);
 }
 
diff --git a/libio/tst-mmap-eofsync.c b/libio/tst-mmap-eofsync.c
index e8ef7271489..0c568e8eb5e 100644
--- a/libio/tst-mmap-eofsync.c
+++ b/libio/tst-mmap-eofsync.c
@@ -60,7 +60,6 @@  do_test (void)
   printf ("feof = %d, ferror = %d immediately after fgets\n",
 	  feof (f), ferror (f));
 
-#if 1
   c = fgetc (f);
   if (c == EOF)
     printf ("fgetc -> EOF (feof = %d, ferror = %d)\n",
@@ -71,7 +70,6 @@  do_test (void)
 	      c, feof (f), ferror (f));
       result = 1;
     }
-#endif
 
   c = write (temp_fd, text2, sizeof text2 - 1);
   if (c == sizeof text2 - 1)
diff --git a/libio/wfileops.c b/libio/wfileops.c
index aecbf0d86d7..8bb4fd78c9f 100644
--- a/libio/wfileops.c
+++ b/libio/wfileops.c
@@ -196,13 +196,9 @@  _IO_wfile_underflow (FILE *fp)
       _IO_wdoallocbuf (fp);
     }
 
-  /* Flush all line buffered files before reading. */
   /* FIXME This can/should be moved to genops ?? */
   if (fp->_flags & (_IO_LINE_BUF | _IO_UNBUFFERED))
     {
-#if 0
-      _IO_flush_all_linebuffered ();
-#else
       /* We used to flush all line-buffered stream.  This really isn't
 	 required by any standard.  My recollection is that
 	 traditional Unix systems did this for stdout.  stderr better
@@ -215,7 +211,6 @@  _IO_wfile_underflow (FILE *fp)
 	_IO_OVERFLOW (_IO_stdout, EOF);
 
       _IO_release_lock (_IO_stdout);
-#endif
     }
 
   _IO_switch_to_get_mode (fp);
diff --git a/libio/wgenops.c b/libio/wgenops.c
index 2d61bc96524..a2413ec20a2 100644
--- a/libio/wgenops.c
+++ b/libio/wgenops.c
@@ -426,26 +426,6 @@  _IO_free_wbackup_area (FILE *fp)
 }
 libc_hidden_def (_IO_free_wbackup_area)
 
-#if 0
-int
-_IO_switch_to_wput_mode (FILE *fp)
-{
-  fp->_wide_data->_IO_write_base = fp->_wide_data->_IO_read_ptr;
-  fp->_wide_data->_IO_write_ptr = fp->_wide_data->_IO_read_ptr;
-  /* Following is wrong if line- or un-buffered? */
-  fp->_wide_data->_IO_write_end = (fp->_flags & _IO_IN_BACKUP
-				   ? fp->_wide_data->_IO_read_end
-				   : fp->_wide_data->_IO_buf_end);
-
-  fp->_wide_data->_IO_read_ptr = fp->_wide_data->_IO_read_end;
-  fp->_wide_data->_IO_read_base = fp->_wide_data->_IO_read_end;
-
-  fp->_flags |= _IO_CURRENTLY_PUTTING;
-  return 0;
-}
-#endif
-
-
 static int
 save_for_wbackup (FILE *fp, wchar_t *end_p)
 {
@@ -624,20 +604,6 @@  _IO_unsave_wmarkers (FILE *fp)
   struct _IO_marker *mark = fp->_markers;
   if (mark)
     {
-#ifdef TODO
-      streampos offset = seekoff (0, ios::cur, ios::in);
-      if (offset != EOF)
-	{
-	  offset += eGptr () - Gbase ();
-	  for ( ; mark != NULL; mark = mark->_next)
-	    mark->set_streampos (mark->_pos + offset);
-	}
-    else
-      {
-	for ( ; mark != NULL; mark = mark->_next)
-	  mark->set_streampos (EOF);
-      }
-#endif
       fp->_markers = 0;
     }
 
diff --git a/stdio-common/vfprintf.c b/stdio-common/vfprintf.c
index 8f0a6f8cf28..ae412e4b844 100644
--- a/stdio-common/vfprintf.c
+++ b/stdio-common/vfprintf.c
@@ -58,7 +58,7 @@ 
 	}								      \
       if (Format == NULL)						      \
 	{								      \
-	  MAYBE_SET_EINVAL;						      \
+	  __set_errno (EINVAL);						      \
 	  return -1;							      \
 	}								      \
     } while (0)
diff --git a/stdio-common/vfscanf.c b/stdio-common/vfscanf.c
index a519a57b8fd..3263268c7ea 100644
--- a/stdio-common/vfscanf.c
+++ b/stdio-common/vfscanf.c
@@ -172,7 +172,7 @@ 
 	}								      \
       else if (format == NULL)						      \
 	{								      \
-	  MAYBE_SET_EINVAL;						      \
+	  __set_errno (EINVAL);						      \
 	  return EOF;							      \
 	}								      \
     } while (0)