[ros-dev] [ros-diffs] [jgardou] 65140: [FASTFAT] - Fix renaming files in case the target file already exists. - Plug a few reference count leaks on FCBs. CORE-8660 #comment fixed in r65140 #resolve CORE-8633 #comment fi...

[Pierre Schweitzer]

Warm thanks for debugging this issue Jérôme. I do value your help.

This should unlock many autoupdating apps in ReactOS.

Thanks again.

On 31/10/2014 12:04, jgardou at svn.reactos.org wrote:
> Author: jgardou
> Date: Fri Oct 31 11:04:12 2014
> New Revision: 65140
> 
> URL: http://svn.reactos.org/svn/reactos?rev=65140&view=rev
> Log:
> [FASTFAT]
>  - Fix renaming files in case the target file already exists.
>  - Plug a few reference count leaks on FCBs.
> CORE-8660 #comment fixed in r65140 #resolve
> CORE-8633 #comment fixed in r65140 #resolve
> CORE-4758 #comment fixed in r65140 #resolve
> 
> Modified:
>     trunk/reactos/drivers/filesystems/fastfat/cleanup.c
>     trunk/reactos/drivers/filesystems/fastfat/create.c
>     trunk/reactos/drivers/filesystems/fastfat/finfo.c
> 
> Modified: trunk/reactos/drivers/filesystems/fastfat/cleanup.c
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/fastfat/cleanup.c?rev=65140&r1=65139&r2=65140&view=diff
> ==============================================================================
> --- trunk/reactos/drivers/filesystems/fastfat/cleanup.c	[iso-8859-1] (original)
> +++ trunk/reactos/drivers/filesystems/fastfat/cleanup.c	[iso-8859-1] Fri Oct 31 11:04:12 2014
> @@ -92,6 +92,7 @@
>                  pFcb->FileObject = NULL;
>                  CcUninitializeCacheMap(tmpFileObject, NULL, NULL);
>                  ObDereferenceObject(tmpFileObject);
> +                vfatReleaseFCB(IrpContext->DeviceExt, pFcb);
>              }
>  
>              CcPurgeCacheSection(FileObject->SectionObjectPointer, NULL, 0, FALSE);
> 
> Modified: trunk/reactos/drivers/filesystems/fastfat/create.c
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/fastfat/create.c?rev=65140&r1=65139&r2=65140&view=diff
> ==============================================================================
> --- trunk/reactos/drivers/filesystems/fastfat/create.c	[iso-8859-1] (original)
> +++ trunk/reactos/drivers/filesystems/fastfat/create.c	[iso-8859-1] Fri Oct 31 11:04:12 2014
> @@ -365,7 +365,6 @@
>          DPRINT("'%wZ'\n", &FileObject->RelatedFileObject->FileName);
>  
>          *ParentFcb = FileObject->RelatedFileObject->FsContext;
> -        vfatGrabFCB(DeviceExt, *ParentFcb);
>      }
>      else
>      {
> 
> Modified: trunk/reactos/drivers/filesystems/fastfat/finfo.c
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/fastfat/finfo.c?rev=65140&r1=65139&r2=65140&view=diff
> ==============================================================================
> --- trunk/reactos/drivers/filesystems/fastfat/finfo.c	[iso-8859-1] (original)
> +++ trunk/reactos/drivers/filesystems/fastfat/finfo.c	[iso-8859-1] Fri Oct 31 11:04:12 2014
> @@ -396,41 +396,53 @@
>      /* If it exists */
>      if (NT_SUCCESS(Status))
>      {
> +        DPRINT("Target file %wZ exists. FCB Flags %08x\n", NewName, TargetFcb->Flags);
>          /* Check whether we are allowed to replace */
>          if (ReplaceIfExists)
>          {
>              /* If that's a directory or a read-only file, we're not allowed */
> -            if (vfatFCBIsDirectory(TargetFcb) || ((*TargetFcb->Attributes & FILE_ATTRIBUTE_READONLY) == FILE_ATTRIBUTE_READONLY));
> -            {
> +            if (vfatFCBIsDirectory(TargetFcb) || ((*TargetFcb->Attributes & FILE_ATTRIBUTE_READONLY) == FILE_ATTRIBUTE_READONLY))
> +            {
> +                DPRINT("And this is a readonly file!\n");
>                  vfatReleaseFCB(DeviceExt, *ParentFCB);
>                  *ParentFCB = NULL;
>                  vfatReleaseFCB(DeviceExt, TargetFcb);
>                  return STATUS_OBJECT_NAME_COLLISION;
>              }
>  
> -            /* Attempt to flush (might close the file) */
> -            if (!MmFlushImageSection(TargetFcb->FileObject->SectionObjectPointer, MmFlushForDelete))
> -            {
> +
> +            /* If we still have a file object, close it. */
> +            if (TargetFcb->FileObject)
> +            {
> +                if (!MmFlushImageSection(TargetFcb->FileObject->SectionObjectPointer, MmFlushForDelete))
> +                {
> +                    DPRINT("MmFlushImageSection failed.\n");
> +                    vfatReleaseFCB(DeviceExt, *ParentFCB);
> +                    *ParentFCB = NULL;
> +                    vfatReleaseFCB(DeviceExt, TargetFcb);
> +                    return STATUS_ACCESS_DENIED;
> +                }
> +
> +                TargetFcb->FileObject->DeletePending = TRUE;
> +                VfatCloseFile(DeviceExt, TargetFcb->FileObject);
> +            }
> +
> +            /* If we are here, ensure the file isn't open by anyone! */
> +            if (TargetFcb->OpenHandleCount != 0)
> +            {
> +                DPRINT("There are still open handles for this file.\n");
>                  vfatReleaseFCB(DeviceExt, *ParentFCB);
>                  *ParentFCB = NULL;
>                  vfatReleaseFCB(DeviceExt, TargetFcb);
>                  return STATUS_ACCESS_DENIED;
>              }
>  
> -            /* If we are, ensure the file isn't open by anyone! */
> -            if (TargetFcb->OpenHandleCount != 0)
> -            {
> -                vfatReleaseFCB(DeviceExt, *ParentFCB);
> -                *ParentFCB = NULL;
> -                vfatReleaseFCB(DeviceExt, TargetFcb);
> -                return STATUS_ACCESS_DENIED;
> -            }
> -
>              /* Effectively delete old file to allow renaming */
> +            DPRINT("Effectively deleting the file.\n");
>              VfatDelEntry(DeviceExt, TargetFcb, NULL);
> -            vfatGrabFCB(DeviceExt, *ParentFCB);
>              vfatReleaseFCB(DeviceExt, TargetFcb);
>              *Deleted = TRUE;
> +            return STATUS_SUCCESS;
>          }
>          else
>          {
> 
> 


-- 
Pierre Schweitzer <pierre at reactos.org>
System & Network Administrator
Senior Kernel Developer
ReactOS Deutschland e.V.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 3968 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://www.reactos.org/pipermail/ros-dev/attachments/20141031/7c271496/attachment-0001.bin>