On Tue, Dec 22, 2020 at 05:30:14PM +0800, Yue Hu wrote:

...

> > > 
> > >   
> > > > +       else if (asprintf(&fspath, "%s/%s", cfg.mount_point,
> > > > +                         erofs_fspath(path)) <= 0)  
> > > 
> > > The argument of path will be root directory. And canned fs_config for 
> > > root directory as
> > > below:
> > > 
> > > 0 0 755 selabel=u:object_r:rootfs:s0 capabilities=0x0
> > > 
> > > So, cannot add mount point to root directory for canned fs_config. And 
> > > what about non-canned
> > > fs_config?  
> > 
> > Not quite sure what you mean. For non-canned fs_config, we didn't observed 
> > any strange
> > before (I ported to cuttlefish and hikey960 with boot success, also as I 
> > mentioned before
> > some other vendors already use it.)
> > 
> > I think the following commit is only useful for squashfs since its 
> > (non)root inode
> > workflows are different, so need to add in two difference place. But 
> > mkfs.erofs is not.
> > https://android.googlesource.com/platform/external/squashfs-tools/+/85a6bc1e52bb911f195c5dc0890717913938c2d1%5E%21/#F0
> > 
> > For root inode is erofs, I think erofs_fspath(path) would return "", so 
> > that case
> > is included as well.... Am I missing something?
> 
> Yes, erofs_fspath(path) returns "" for root inode. However, the above patch 
> add the mount
> point to fspath when specify it, so the real path is "vendor/" which does not 
> exist in canned
> fs_config file. build will report below error:
> 
> failed to find [/vendor/] in canned fs_config

Hmmm... such design is quite strange for me....
Could you try the following diff?

diff --git a/lib/inode.c b/lib/inode.c
index 9469074..9af6179 100644
--- a/lib/inode.c
+++ b/lib/inode.c
@@ -698,11 +698,14 @@ int erofs_droid_inode_fsconfig(struct erofs_inode *inode,
        /* filesystem_config does not preserve file type bits */
        mode_t stat_file_type_mask = st->st_mode & S_IFMT;
        unsigned int uid = 0, gid = 0, mode = 0;
+       bool alloced;
        char *fspath;
 
        inode->capabilities = 0;
-       if (!cfg.mount_point)
-               fspath = erofs_fspath(path);
+
+       alloced = (cfg.mount_point && erofs_fspath(path)[0] != '\0');
+       if (!alloced)
+               fspath = (char *)erofs_fspath(path);
        else if (asprintf(&fspath, "%s/%s", cfg.mount_point,
                          erofs_fspath(path)) <= 0)
                return -ENOMEM;
@@ -718,7 +721,7 @@ int erofs_droid_inode_fsconfig(struct erofs_inode *inode,
                          cfg.target_out_path,
                          &uid, &gid, &mode, &inode->capabilities);
 
-       if (cfg.mount_point)
+       if (alloced)
                free(fspath);
        st->st_uid = uid;
        st->st_gid = gid;

if it works, will redo a formal patch then....

Thanks,
Gao Xiang

> 
> > 
> > Thanks,
> > Gao Xiang
> > 
> > > 
> > > Thx.
> > > 
> > >   
> > > > +               return -ENOMEM;
> > > > +
> > > > +
> > > >         if (cfg.fs_config_file)
> > > > -               canned_fs_config(erofs_fspath(path),
> > > > +               canned_fs_config(fspath,
> > > >                                  S_ISDIR(st->st_mode),
> > > >                                  cfg.target_out_path,
> > > >                                  &uid, &gid, &mode, 
> > > > &inode->capabilities);
> > > > -       else if (cfg.mount_point) {
> > > > -               if (asprintf(&fspath, "%s/%s", cfg.mount_point,
> > > > -                            erofs_fspath(path)) <= 0)
> > > > -                       return -ENOMEM;
> > > > -
> > > > +       else
> > > >                 fs_config(fspath, S_ISDIR(st->st_mode),
> > > >                           cfg.target_out_path,
> > > >                           &uid, &gid, &mode, &inode->capabilities);
> > > > +
> > > > +       if (cfg.mount_point)
> > > >                 free(fspath);
> > > > -       }
> > > >         st->st_uid = uid;
> > > >         st->st_gid = gid;
> > > >         st->st_mode = mode | stat_file_type_mask;  
> > >   
> > 
> 

Reply via email to