| Message ID | 20201222135849.38052-3-kieran.bingham@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 0a823785fad27e1eb9a900e80923bc9bde106de9 |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thank you for the patch. On Tue, Dec 22, 2020 at 01:58:49PM +0000, Kieran Bingham wrote: > Ensure that when we check for existence with File() it will only > return if the path leads to a file, and not a directory. > > Device nodes which could still be opened by this class are still supported. Sometimes I wonder if text editors also have their moments of inattention and forget to wrap some lines :-) > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/libcamera/file.cpp | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/file.cpp b/src/libcamera/file.cpp > index 3a3f5bb63ffc..bce2b6138239 100644 > --- a/src/libcamera/file.cpp > +++ b/src/libcamera/file.cpp > @@ -458,7 +458,8 @@ bool File::exists(const std::string &name) > if (ret < 0) > return false; > > - return true; > + /* Directories can not be handled here, even if they exist. */ > + return !S_ISDIR(st.st_mode); I initially thought that we would still return true for a symlink pointing to a directory, but stat() dereferences symlinks, so I think we're good. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > } > > } /* namespace libcamera */
Hi Laurent, On 22/12/2020 15:57, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patch. > > On Tue, Dec 22, 2020 at 01:58:49PM +0000, Kieran Bingham wrote: >> Ensure that when we check for existence with File() it will only >> return if the path leads to a file, and not a directory. >> >> Device nodes which could still be opened by this class are still supported. Ah, I write most of my commit messages on the command line - git commit -m "..." I'll wrap before pushing. Thanks Kieran > > Sometimes I wonder if text editors also have their moments of > inattention and forget to wrap some lines :-) > >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> --- >> src/libcamera/file.cpp | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/src/libcamera/file.cpp b/src/libcamera/file.cpp >> index 3a3f5bb63ffc..bce2b6138239 100644 >> --- a/src/libcamera/file.cpp >> +++ b/src/libcamera/file.cpp >> @@ -458,7 +458,8 @@ bool File::exists(const std::string &name) >> if (ret < 0) >> return false; >> >> - return true; >> + /* Directories can not be handled here, even if they exist. */ >> + return !S_ISDIR(st.st_mode); > > I initially thought that we would still return true for a symlink > pointing to a directory, but stat() dereferences symlinks, so I think > we're good. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> } >> >> } /* namespace libcamera */ >
On 22.12.2020 17:57, Laurent Pinchart wrote: >Hi Kieran, > >Thank you for the patch. > >On Tue, Dec 22, 2020 at 01:58:49PM +0000, Kieran Bingham wrote: >> Ensure that when we check for existence with File() it will only >> return if the path leads to a file, and not a directory. >> >> Device nodes which could still be opened by this class are still supported. > >Sometimes I wonder if text editors also have their moments of >inattention and forget to wrap some lines :-) I noticed that this can happen when I paste some long line from the system clipboard into VIM. In that case it doesn't automatically wrap the text. > >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> --- >> src/libcamera/file.cpp | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/src/libcamera/file.cpp b/src/libcamera/file.cpp >> index 3a3f5bb63ffc..bce2b6138239 100644 >> --- a/src/libcamera/file.cpp >> +++ b/src/libcamera/file.cpp >> @@ -458,7 +458,8 @@ bool File::exists(const std::string &name) >> if (ret < 0) >> return false; >> >> - return true; >> + /* Directories can not be handled here, even if they exist. */ >> + return !S_ISDIR(st.st_mode); > >I initially thought that we would still return true for a symlink >pointing to a directory, but stat() dereferences symlinks, so I think >we're good. > >Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> } >> >> } /* namespace libcamera */ > >-- >Regards, > >Laurent Pinchart >_______________________________________________ >libcamera-devel mailing list >libcamera-devel@lists.libcamera.org >https://lists.libcamera.org/listinfo/libcamera-devel
Hi Kieran, On 2020-12-22 13:58:49 +0000, Kieran Bingham wrote: > Ensure that when we check for existence with File() it will only > return if the path leads to a file, and not a directory. > > Device nodes which could still be opened by this class are still supported. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> With the long line already pointed out fixed, Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/file.cpp | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/file.cpp b/src/libcamera/file.cpp > index 3a3f5bb63ffc..bce2b6138239 100644 > --- a/src/libcamera/file.cpp > +++ b/src/libcamera/file.cpp > @@ -458,7 +458,8 @@ bool File::exists(const std::string &name) > if (ret < 0) > return false; > > - return true; > + /* Directories can not be handled here, even if they exist. */ > + return !S_ISDIR(st.st_mode); > } > > } /* namespace libcamera */ > -- > 2.25.1 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/src/libcamera/file.cpp b/src/libcamera/file.cpp index 3a3f5bb63ffc..bce2b6138239 100644 --- a/src/libcamera/file.cpp +++ b/src/libcamera/file.cpp @@ -458,7 +458,8 @@ bool File::exists(const std::string &name) if (ret < 0) return false; - return true; + /* Directories can not be handled here, even if they exist. */ + return !S_ISDIR(st.st_mode); } } /* namespace libcamera */
Ensure that when we check for existence with File() it will only return if the path leads to a file, and not a directory. Device nodes which could still be opened by this class are still supported. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- src/libcamera/file.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)