Re: Use FREF() & finishdup() in dupfdopen()

[Alexander Bluhm]

On Tue, Apr 03, 2018 at 04:43:17PM +0200, Martin Pieuchot wrote:
> Here's another refactoring to properly reference count a 'struct file *'
> just after calling fd_getfile().
> 
> Instead of incrementing `f_count' by hand, give the current reference
> to finishdup() like it is done in other places in the same file.
> 
> Ok?

OK bluhm@

> Index: kern/kern_descrip.c
> ===
> RCS file: /cvs/src/sys/kern/kern_descrip.c,v
> retrieving revision 1.144
> diff -u -p -r1.144 kern_descrip.c
> --- kern/kern_descrip.c   3 Apr 2018 09:00:03 -   1.144
> +++ kern/kern_descrip.c   3 Apr 2018 13:30:47 -
> @@ -1274,6 +1274,8 @@ dupfdopen(struct proc *p, int indx, int 
>   struct filedesc *fdp = p->p_fd;
>   int dupfd = p->p_dupfd;
>   struct file *wfp;
> + int error, flags;
> + register_t dummy;
>  
>   fdpassertlocked(fdp);
>  
> @@ -1297,24 +1299,26 @@ dupfdopen(struct proc *p, int indx, int 
>*/
>   if ((wfp = fd_getfile(fdp, dupfd)) == NULL)
>   return (EBADF);
> + FREF(wfp);
>  
>   /*
>* Check that the mode the file is being opened for is a
>* subset of the mode of the existing descriptor.
>*/
> - if (((mode & (FREAD|FWRITE)) | wfp->f_flag) != wfp->f_flag)
> + if (((mode & (FREAD|FWRITE)) | wfp->f_flag) != wfp->f_flag) {
> + FRELE(wfp, p);
>   return (EACCES);
> - if (wfp->f_count == LONG_MAX-2)
> - return (EDEADLK);
> + }
>  
> - fdp->fd_ofiles[indx] = wfp;
> - fdp->fd_ofileflags[indx] = (fdp->fd_ofileflags[indx] & UF_EXCLOSE) |
> - (fdp->fd_ofileflags[dupfd] & ~UF_EXCLOSE);
> + flags = fdp->fd_ofileflags[indx] & UF_EXCLOSE;
>   if (ISSET(p->p_p->ps_flags, PS_PLEDGE))
> - fdp->fd_ofileflags[indx] |= UF_PLEDGED;
> - wfp->f_count++;
> - fd_used(fdp, indx);
> - return (0);
> + flags |= UF_PLEDGED;
> +
> + /* finishdup() does FRELE */
> + error = finishdup(p, wfp, dupfd, indx, , 1);
> + if (error == 0)
> + fdp->fd_ofileflags[indx] |= flags;
> + return (error);
>  }
>  
>  /*

Use FREF() & finishdup() in dupfdopen()

[Martin Pieuchot]

Here's another refactoring to properly reference count a 'struct file *'
just after calling fd_getfile().

Instead of incrementing `f_count' by hand, give the current reference
to finishdup() like it is done in other places in the same file.

Ok?

Index: kern/kern_descrip.c
===
RCS file: /cvs/src/sys/kern/kern_descrip.c,v
retrieving revision 1.144
diff -u -p -r1.144 kern_descrip.c
--- kern/kern_descrip.c 3 Apr 2018 09:00:03 -   1.144
+++ kern/kern_descrip.c 3 Apr 2018 13:30:47 -
@@ -1274,6 +1274,8 @@ dupfdopen(struct proc *p, int indx, int 
struct filedesc *fdp = p->p_fd;
int dupfd = p->p_dupfd;
struct file *wfp;
+   int error, flags;
+   register_t dummy;
 
fdpassertlocked(fdp);
 
@@ -1297,24 +1299,26 @@ dupfdopen(struct proc *p, int indx, int 
 */
if ((wfp = fd_getfile(fdp, dupfd)) == NULL)
return (EBADF);
+   FREF(wfp);
 
/*
 * Check that the mode the file is being opened for is a
 * subset of the mode of the existing descriptor.
 */
-   if (((mode & (FREAD|FWRITE)) | wfp->f_flag) != wfp->f_flag)
+   if (((mode & (FREAD|FWRITE)) | wfp->f_flag) != wfp->f_flag) {
+   FRELE(wfp, p);
return (EACCES);
-   if (wfp->f_count == LONG_MAX-2)
-   return (EDEADLK);
+   }
 
-   fdp->fd_ofiles[indx] = wfp;
-   fdp->fd_ofileflags[indx] = (fdp->fd_ofileflags[indx] & UF_EXCLOSE) |
-   (fdp->fd_ofileflags[dupfd] & ~UF_EXCLOSE);
+   flags = fdp->fd_ofileflags[indx] & UF_EXCLOSE;
if (ISSET(p->p_p->ps_flags, PS_PLEDGE))
-   fdp->fd_ofileflags[indx] |= UF_PLEDGED;
-   wfp->f_count++;
-   fd_used(fdp, indx);
-   return (0);
+   flags |= UF_PLEDGED;
+
+   /* finishdup() does FRELE */
+   error = finishdup(p, wfp, dupfd, indx, , 1);
+   if (error == 0)
+   fdp->fd_ofileflags[indx] |= flags;
+   return (error);
 }
 
 /*