[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 16/18] mini-os: reset file type in close() in one place only



On 11.01.22 19:11, Andrew Cooper wrote:
On 11/01/2022 14:58, Juergen Gross wrote:
diff --git a/lib/sys.c b/lib/sys.c
index 0e6fe5d..323a7cd 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -424,87 +424,82 @@ int fsync(int fd) {
int close(int fd)
  {
+    int res = 0;
+
      printk("close(%d)\n", fd);
      switch (files[fd].type) {
          default:
-           files[fd].type = FTYPE_NONE;
-           return 0;
+            break;
  #ifdef CONFIG_XENBUS
        case FTYPE_XENBUS:
              xs_daemon_close((void*)(intptr_t) fd);
-            return 0;
+            break;
  #endif
  #ifdef HAVE_LWIP
-       case FTYPE_SOCKET: {
-           int res = lwip_close(files[fd].fd);
-           files[fd].type = FTYPE_NONE;
-           return res;
-       }
+       case FTYPE_SOCKET:
+           res = lwip_close(files[fd].fd);

Hard tabs.

+            break;
  #endif
  #ifdef CONFIG_LIBXENCTRL
        case FTYPE_XC:
            minios_interface_close_fd(fd);
-           return 0;
+            break;
  #endif
  #ifdef CONFIG_LIBXENEVTCHN
        case FTYPE_EVTCHN:
            minios_evtchn_close_fd(fd);
-            return 0;
+            break;
  #endif
  #ifdef CONFIG_LIBXENGNTTAB
        case FTYPE_GNTMAP:
            minios_gnttab_close_fd(fd);
-           return 0;
+            break;
  #endif
  #ifdef CONFIG_NETFRONT
        case FTYPE_TAP:
            shutdown_netfront(files[fd].dev);
-           files[fd].type = FTYPE_NONE;
-           return 0;
+            break;
  #endif
  #ifdef CONFIG_BLKFRONT
        case FTYPE_BLK:
              shutdown_blkfront(files[fd].dev);
-           files[fd].type = FTYPE_NONE;
-           return 0;
+            break;
  #endif
  #ifdef CONFIG_TPMFRONT
        case FTYPE_TPMFRONT:
              shutdown_tpmfront(files[fd].dev);
-           files[fd].type = FTYPE_NONE;
-           return 0;
+            break;
  #endif
  #ifdef CONFIG_TPM_TIS
        case FTYPE_TPM_TIS:
              shutdown_tpm_tis(files[fd].dev);
-           files[fd].type = FTYPE_NONE;
-           return 0;
+            break;
  #endif
  #ifdef CONFIG_KBDFRONT
        case FTYPE_KBD:
              shutdown_kbdfront(files[fd].dev);
-            files[fd].type = FTYPE_NONE;
-            return 0;
+            break;
  #endif
  #ifdef CONFIG_FBFRONT
        case FTYPE_FB:
              shutdown_fbfront(files[fd].dev);
-            files[fd].type = FTYPE_NONE;
-            return 0;
+            break;
  #endif
  #ifdef CONFIG_CONSFRONT
          case FTYPE_SAVEFILE:
          case FTYPE_CONSOLE:
              fini_consfront(files[fd].dev);
-            files[fd].type = FTYPE_NONE;
-            return 0;
+            break;
  #endif
        case FTYPE_NONE:
-           break;
+            printk("close(%d): Bad descriptor\n", fd);
+            errno = EBADF;
+            return -1;
      }
-    printk("close(%d): Bad descriptor\n", fd);
-    errno = EBADF;
-    return -1;
+
+    memset(files + fd, 0, sizeof(struct file));
+    files[fd].type = FTYPE_NONE;

BUILD_BUG_ON(FTYPE_NONE != 0);

Life's too short to deal with a theoretical (and short sighted) future
where someone might want to change FTYPE_NONE away from being 0.

Good idea.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.