X-Authentication-Warning: delorie.com: mailnull set sender to djgpp-workers-bounces using -f
Date: Wed, 03 Apr 2002 18:22:36 +0100
From: "Richard Dawe" <rich AT phekda DOT freeserve DOT co DOT uk>
Sender: rich AT phekda DOT freeserve DOT co DOT uk
To: djgpp-workers AT delorie DOT com
X-Mailer: Emacs 21.2.50 (via feedmail 8.3.emacs20_6 I) and Blat ver 1.8.6
Subject: FSEXT: grow_table and realloc failure
Message-Id: <E16soP7-0000oI-00@phekda.freeserve.co.uk>
Reply-To: djgpp-workers AT delorie DOT com

Hello.

Below is a diff to fix the realloc bug in grow_table in the FSEXT code.
If realloc fails, then the current code will trash the __FSEXT_entry
table. This can lead to bogus behaviour when the program exits
and the __FSEXT_close_all function is called. On one occassion
it wedged my box. Here is a changelog entry, which I will add to
the "What's Changed" file:

    @cindex FSEXT memory leaks
    A memory leak under low-memory conditions was fixed in
    the File System Extension code.

OK to commit?

If anyone else wants to test out the changes, below is the test program
I used. To use it, change chunk_sz in fsext.c to, say, 16 and add
code to use a different memory allocator in grow_table, after
a certain number of fds have been allocated.

Bye, Rich

/*
 * manyfds.c
 *
 * Check that FSEXT mechanism can cope, when realloc fails.
 * The grow_table function in src/libc/fsext/fsext.c should not
 * through away state for the currently hooked file descriptors.
 */ 

/* Test written by Richard Dawe <richdawe AT bigfoot DOT com> */

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <sys/fsext.h>

/* Active number of file descriptors */
static int n_fds = 0;

static int
manyfds_fsext (__FSEXT_Fnumber func_number, int *rv, va_list args)
{
  int ret = 0; /* Not emulated by default */

  switch(func_number) {
  case __FSEXT_close:
    *rv = 0; /* OK */
    n_fds--;
    ret = 1; /* Handled */
    break;

  default:
    break;
  }

  return(ret);
}

/* This should cause grow_table to be called. */
#define MAX_N_FDS 103

int
main (void)
{
  const int max_n_fds = MAX_N_FDS;
  int fds[MAX_N_FDS];
  int i, i_max, fd;

  memset(fds, 0, sizeof(fds));

  for (i = 0; i < max_n_fds; i++) {
    fd = __FSEXT_alloc_fd(manyfds_fsext);

    if (fd < 0) {
      fprintf(stderr,
	      "Unable to allocate file descriptor %d: %s\n",
	      i, strerror(errno));
      break;
    }

    fds[i] = fd;
    n_fds++;
  }

  printf("Allocated %d file descriptors\n", n_fds);

  /* Clean up */
  i_max = n_fds; /* n_fds may be changed by close(). */
  for (i = 0; i < i_max; i++) { close(fds[i]); }

  if (n_fds == 0) {
    fprintf(stderr, "PASS - all freed\n");
    return(EXIT_SUCCESS);
  }

  fprintf(stderr,
	  "FAIL - less freed than allocated - %d allocated, %d freed\n",
	  max_n_fds, max_n_fds - n_fds);

  return(EXIT_FAILURE);
}

Index: src/libc/fsext/fsext.c
===================================================================
RCS file: /cvs/djgpp/djgpp/src/libc/fsext/fsext.c,v
retrieving revision 1.3
diff -p -u -3 -r1.3 fsext.c
--- src/libc/fsext/fsext.c	2000/06/29 08:37:09	1.3
+++ src/libc/fsext/fsext.c	2002/04/03 17:12:53
@@ -1,3 +1,4 @@
+/* Copyright (C) 2002 DJ Delorie, see COPYING.DJ for details */
 /* Copyright (C) 2000 DJ Delorie, see COPYING.DJ for details */
 /* Copyright (C) 1998 DJ Delorie, see COPYING.DJ for details */
 /* Copyright (C) 1995 DJ Delorie, see COPYING.DJ for details */
@@ -81,6 +82,9 @@ __FSEXT_alloc_fd(__FSEXT_Function *_func
 static int
 grow_table(int _fd)
 {
+  /* Allocate table in chunks of chunk_sz */
+  const int chunk_sz = 1<<8; /* 256 */
+
   init();
 
   if (_fd < 0)
@@ -88,11 +92,20 @@ grow_table(int _fd)
 
   if (num_fds <= _fd)
   {
+    __FSEXT_entry *temp;
     int old_fds = num_fds, i;
-    num_fds = (_fd+256) & ~255;
-    fsext_list = (__FSEXT_entry *)realloc(fsext_list, num_fds * sizeof(__FSEXT_entry));
-    if (fsext_list == 0)
+
+    num_fds = (_fd+chunk_sz) & ~(chunk_sz-1);
+    temp = realloc(fsext_list, num_fds * sizeof(__FSEXT_entry));
+    if (temp == 0)
+    {
+      /* Keep the current fsext_list, so that we can tidy the FSEXTs
+	 up properly. */
+      num_fds = old_fds;
       return 1;
+    }
+
+    fsext_list = temp;
     for (i=old_fds; i<num_fds; i++)
     {
       fsext_list[i].function = 0;