Question about tmpfs

2013-10-30 Thread Maxime Villard 

Hi,
I have a question regarding the function tmpfs_alloc_node() in
fs/tmpfs/tmpfs_subr.c. When alloc'ing the area for symlinks,
there's this code:

l.171
nnode->tn_size = strlen(target);
if (nnode->tn_size == 0) {
nnode->tn_spec.tn_lnk.tn_link = NULL;
break;
}
nnode->tn_spec.tn_lnk.tn_link =
tmpfs_strname_alloc(tmp, nnode->tn_size);
if (nnode->tn_spec.tn_lnk.tn_link == NULL) {
tmpfs_node_put(tmp, nnode);
return ENOSPC;
}
memcpy(nnode->tn_spec.tn_lnk.tn_link, target, nnode->tn_size);

Only strlen(target) bytes are allocated for 'target', and only
strlen(target) bytes are copied from 'target' to 'tn_link'.

Why isn't the '\0' taken into account? Is this intentional?

Thanks.

Re: Question about tmpfs

2013-10-30 Thread Christos Zoulas 
In article <52713dff.2000...@m00nbsd.net>,
Maxime Villard   wrote:
>Hi,
>I have a question regarding the function tmpfs_alloc_node() in
>fs/tmpfs/tmpfs_subr.c. When alloc'ing the area for symlinks,
>there's this code:
>
>l.171
>   nnode->tn_size = strlen(target);
>   if (nnode->tn_size == 0) {
>   nnode->tn_spec.tn_lnk.tn_link = NULL;
>   break;
>   }
>   nnode->tn_spec.tn_lnk.tn_link =
>   tmpfs_strname_alloc(tmp, nnode->tn_size);
>   if (nnode->tn_spec.tn_lnk.tn_link == NULL) {
>   tmpfs_node_put(tmp, nnode);
>   return ENOSPC;
>   }
>   memcpy(nnode->tn_spec.tn_lnk.tn_link, target, nnode->tn_size);
>
>Only strlen(target) bytes are allocated for 'target', and only
>strlen(target) bytes are copied from 'target' to 'tn_link'.
>
>Why isn't the '\0' taken into account? Is this intentional?

I don't think it is from a quick reading. The only reason it works,
is because most of the time it rounds up.

christos

Re: Question about tmpfs

2013-10-30 Thread tsugutomo . enami 
> I don't think it is from a quick reading. The only reason it works,
> is because most of the time it rounds up.

It looks like the only place where the member is used is here:

   tmpfs_vnops.c:  error = uiomove(node->tn_spec.tn_lnk.tn_link,
   tmpfs_vnops.c-  MIN(node->tn_size, uio->uio_resid), uio);

Is there any other use?

enami.

Re: Question about tmpfs

2013-10-30 Thread Mindaugas Rasiukevicius 
chris...@astron.com (Christos Zoulas) wrote:
> In article <52713dff.2000...@m00nbsd.net>,
> Maxime Villard   wrote:
> >Hi,
> >I have a question regarding the function tmpfs_alloc_node() in
> >fs/tmpfs/tmpfs_subr.c. When alloc'ing the area for symlinks,
> >there's this code:
> >
> >l.171
> > nnode->tn_size = strlen(target);
> > if (nnode->tn_size == 0) {
> > nnode->tn_spec.tn_lnk.tn_link = NULL;
> > break;
> > }
> > nnode->tn_spec.tn_lnk.tn_link =
> > tmpfs_strname_alloc(tmp, nnode->tn_size);
> > if (nnode->tn_spec.tn_lnk.tn_link == NULL) {
> > tmpfs_node_put(tmp, nnode);
> > return ENOSPC;
> > }
> > memcpy(nnode->tn_spec.tn_lnk.tn_link, target, nnode->tn_size);
> >
> >Only strlen(target) bytes are allocated for 'target', and only
> >strlen(target) bytes are copied from 'target' to 'tn_link'.
> >
> >Why isn't the '\0' taken into account? Is this intentional?
> 
> I don't think it is from a quick reading. The only reason it works,
> is because most of the time it rounds up.

It is not a bug, but it is potentially error-prone.  I adjusted the code:

http://mail-index.netbsd.org/source-changes/2013/10/31/msg048829.html

-- 
Mindaugas

Re: Question about tmpfs

2013-10-31 Thread Maxime Villard 

Le 31/10/2013 02:03, Mindaugas Rasiukevicius a écrit :

It is not a bug, but it is potentially error-prone.  I adjusted the code:

http://mail-index.netbsd.org/source-changes/2013/10/31/msg048829.html


Ok. While I'm at it: tmpfs_read() returns EISDIR when the file
is not regular, but a non-regular file is not necessarily a dir.
The code is now consistent with tmpfs_write(). Also, a typo in a
comment.


Index: tmpfs_subr.c
===
RCS file: /cvsroot/src/sys/fs/tmpfs/tmpfs_subr.c,v
retrieving revision 1.81
diff -u -r1.81 tmpfs_subr.c
--- tmpfs_subr.c31 Oct 2013 00:59:17 -  1.81
+++ tmpfs_subr.c31 Oct 2013 06:46:48 -
@@ -332,7 +332,7 @@
 }

 /*
- * tmpfs_alloc_file: allocate a new file of specified type and adds it
+ * tmpfs_alloc_file: allocate a new file of specified type and add it
  * into the parent directory.
  *
  * => Credentials of the caller are used.
Index: tmpfs_vnops.c
===
RCS file: /cvsroot/src/sys/fs/tmpfs/tmpfs_vnops.c,v
retrieving revision 1.104
diff -u -r1.104 tmpfs_vnops.c
--- tmpfs_vnops.c   31 Oct 2013 00:59:17 -  1.104
+++ tmpfs_vnops.c   31 Oct 2013 06:46:48 -
@@ -536,10 +536,7 @@

KASSERT(VOP_ISLOCKED(vp));

-   if (vp->v_type != VREG) {
-   return EISDIR;
-   }
-   if (uio->uio_offset < 0) {
+   if (uio->uio_offset < 0 || vp->v_type != VREG) {
return EINVAL;
}


Ok/Comments?