lib/libc/unmount.c

[Eugeny S. Mints]

Hi, All!

Could you please comment rtems_filesystem_freenode() call( line 132 in
lib/libc/unmount.c) for &fs_root_loc after
temp_mt_entry.mt_fs_root.ops->fsunmount_me_h() call( line 115 in
lib/libc/unmount.c)?( I use rtems-ss-20010816)

As I understand, fsunmount_me_h() provides to mounted fs a chance to make full
clean up activity (i.e. to free all memory, delete semaphores if mounted fs used
any, etc). If it right, then after this moment nobody can call any internal
routines of mounted(but really already unmounted at this moment) fs, am I
right? But rtems_filesystem_freenode does.

For a example, in my case rtems_filesystem_freenode() calls internal
routine which always use an internal semaphore ( because it
is necessary while fs is really mounted ) but this semaphore is deleted
during  fsunmount_me_h() call a and hence no longer exists during
rtems_filesystem_freenode() call( line 132 in
lib/libc/unmount.c).

Anyway, it seems the main idea is that nobody can call any
internal routines of an fs after fsunmount_me_h() call for
this fs.

As to Mr. Till Straumann patch (attached to the letter
"fs core fixes (was Re: TFTP driver changes)"  Date: Wed, 26 Sep 2001
15:40:32 -0700): I didn;t look carefully all files he proposed to patch, but
as to umnount.c, it seems that in line 134 of patched unmount.c he tries to
free the same as in line 136, because it seems to me, that
&fs_root_loc.mt_entry->mt_fs_root and &fs_root_loc points to the same memory
area. At the same time additions rtems_filesystem_freenode(fs_root_loc) in
lines 177 and 199 of patched file seem right.

So, please see my proposal in patch (patch to the current unmount.c - not
patched by Till Straumann' patch) atteched.

Sincerely yours,
				Eugeny

-- 
Eugeny S. Mints
OKTET Ltd.
1 Ulianovskaya st., Petergof, St.Petersburg, 198904 Russia
Phone: +7(812)428-4384 Fax: +7(812)327-2246
mailto:Eugeny.Mints at oktet.ru
-------------- next part --------------
--- unmount.c.orig	Thu Oct 11 20:20:52 2001
+++ unmount.c	Thu Oct 11 20:30:02 2001
@@ -9,6 +9,8 @@
  *  COPYRIGHT (c) 1989-1999.
  *  On-Line Applications Research Corporation (OAR).
  *
+ *  Modified by Eugeny S. Mints <Eugeny.Mints at oktet.ru>
+ *
  *  The license and distribution terms for this file may be
  *  found in the file LICENSE in this distribution or at
  *  http://www.OARcorp.com/rtems/license.html.
@@ -61,7 +63,7 @@
   const char *path
 )
 {
-  int                                   status;
+  int                                   status = 0;
   rtems_filesystem_location_info_t      fs_root_loc;
   rtems_filesystem_mount_table_entry_t  temp_mt_entry;
 
@@ -99,24 +101,27 @@
     set_errno_and_return_minus_one( EBUSY );
   }
   
+  rtems_filesystem_freenode( &fs_root_loc );
+
   /*
-   * Allow the file system being unmounted on to do its cleanup.
+   * Run the unmount function for the subordinate file system.
    */
 
-  if ((temp_mt_entry.mt_point_node.ops->unmount_h )( fs_root_loc.mt_entry ) != 0 ) {
-    rtems_filesystem_freenode( &fs_root_loc );
-    return -1;
+  if ((temp_mt_entry.mt_fs_root.ops->fsunmount_me_h )( fs_root_loc.mt_entry ) != 0){
+    /* don't return because no sensible undo action */
+    status = -1; 
   }
 
   /*
-   * Run the unmount function for the subordinate file system.
+   * Allow the file system being unmounted on to do its cleanup.
    */
 
-  if ((temp_mt_entry.mt_fs_root.ops->fsunmount_me_h )( fs_root_loc.mt_entry ) != 0){
-    rtems_filesystem_freenode( &fs_root_loc );
-    return -1;
+  if ((temp_mt_entry.mt_point_node.ops->unmount_h )( fs_root_loc.mt_entry ) != 0 ) {
+    /* don't return because no sensible undo action */
+    status = -1;
   }
 
+
   /*
    *  Extract the mount table entry from the chain
    */
@@ -129,9 +134,8 @@
 
   rtems_filesystem_freenode( &fs_root_loc.mt_entry->mt_point_node );
   free( fs_root_loc.mt_entry );
-  rtems_filesystem_freenode( &fs_root_loc );
 
-  return 0;
+  return status;
 }
 
 
@@ -169,7 +173,10 @@
 
   *fs_to_unmount = *fs_root_loc->mt_entry;
   if ( fs_to_unmount->mt_fs_root.node_access != fs_root_loc->node_access )
+  {
+    rtems_filesystem_freenode( fs_root_loc );
     set_errno_and_return_minus_one( EACCES );
+  }  
 
   /*
    * Search the mount table for any mount entries referencing this
@@ -180,7 +187,9 @@
         !Chain_Is_tail( &rtems_filesystem_mount_table_control, the_node );
         the_node = the_node->next ) {
      the_mount_entry = ( rtems_filesystem_mount_table_entry_t * )the_node;
-     if (the_mount_entry->mt_point_node.mt_entry  == fs_root_loc->mt_entry ) {
+     if (the_mount_entry->mt_point_node.mt_entry  == fs_root_loc->mt_entry ) 
+     {
+          rtems_filesystem_freenode( fs_root_loc );
           set_errno_and_return_minus_one( EBUSY );
      }
   }