On Mon, Oct 07, 2024 at 04:19:21PM +0200, Amir Goldstein wrote:
> +static int ovl_upper_fdget(const struct file *file, struct fd *real, bool data)
> +{
> + struct dentry *dentry = file_dentry(file);
> + struct path realpath;
> + enum ovl_path_type type;
> +
> + if (data)
> + type = ovl_path_realdata(dentry, &realpath);
> + else
> + type = ovl_path_real(dentry, &realpath);
> +
> + real->word = 0;
> + /* Not interested in lower nor in upper meta if data was requested */
> + if (!OVL_TYPE_UPPER(type) || (data && OVL_TYPE_MERGE(type)))
> + return 0;
> +
> + return ovl_real_fdget_path(file, real, &realpath);
> }
>
> static int ovl_open(struct inode *inode, struct file *file)
> @@ -394,16 +411,14 @@ static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync)
> if (ret <= 0)
> return ret;
>
> - ret = ovl_real_fdget_meta(file, &real, !datasync);
> - if (ret)
> + /* Don't sync lower file for fear of receiving EROFS error */
> + ret = ovl_upper_fdget(file, &real, datasync);
> + if (ret || fd_empty(real))
> return ret;
Is there any real point in keeping ovl_upper_fdget() separate from the
only caller? Note that the checks for type make a lot more sense
in ovl_fsync() than buried in a separate helper and this fd_empty()
is a "do we have the wrong type?" check in disguise.
Why not just expand it at the call site?
