Re: register_driver with 0000 mode

2022-04-01 Thread Petro Karashchenko
Hi,

I want to resume this thread again because after reexamined code carefully
I found that VFS layer has an API

int inode_checkflags(FAR struct inode *inode, int oflags)
{
  if (((oflags & O_RDOK) != 0 && !inode->u.i_ops->read) ||
  ((oflags & O_WROK) != 0 && !inode->u.i_ops->write))
{
  return -EACCES;
}
  else
{
  return OK;
}
}

That checks if read and write handlers are available, so all our discussion
about R/W mode for IOCTL does not make any sense. We either need to remove
this check or register VFS nodes with proper permissions and open files
with correct flags. So if the driver does not have neither read nor write
handlers the "" mode should be used and "0" should be used during
opening of a file. Or we need to remove "inode_checkflags()".

Best regards,
Petro

пт, 28 січ. 2022 р. о 15:11 Petro Karashchenko 
пише:

> I see. Thank you for the feedback. I will rework changes to get back
> read permissions.
>
> Best regards,
> Petro
>
> пт, 28 січ. 2022 р. о 14:41 Alan Carvalho de Assis 
> пише:
> >
> > Hi Petro,
> >
> > The read permission is needed even when you just want to open a file:
> >
> > $ vim noreadfile
> >
> > $ chmod  noreadfile
> >
> > $ ls -l noreadfile
> > -- 1 user user 5 jan 28 09:24 noreadfile
> >
> > $ cat noreadfile
> > cat: noreadfile: Permission denied
> >
> > You can even try to create a C program just to open it, and it will fail.
> >
> > See the man page of open function:
> >
> >The argument flags *must* include one of the  following  access
> >  modes:  O_RDONLY,  O_WRONLY,  or
> >O_RDWR.  These request opening the file read-only, write-only,
> > or read/write, respectively.
> >
> > This man page makes it clear you must include an access mode, but I
> > passed 0 to the access mode flag of open() and it was accepted, but
> > when the file has permission  it returns -EPERM: "Failed to open
> > file: error -1"
> >
> > BR,
> >
> > Alan
> >
> > On 1/28/22, Petro Karashchenko  wrote:
> > > Hello,
> > >
> > > Yes, but how does this relate to "" mode for "register_driver()"?
> > > Maybe you can describe some use case so it will become more clear?
> > > Currently ioctl works fine if driver is registered with ""
> permission
> > > mode.
> > >
> > > Best regards,
> > > Petro
> > >
> > > пт, 28 січ. 2022 р. о 11:39 Xiang Xiao 
> пише:
> > >>
> > >> If we want to do the correct permission check, the ioctl handler
> needs to
> > >> check R/W bit by itself based on how the ioctl is implemented.
> > >> Or follow up how Linux encode the needed permission into each IOCTL:
> > >>
> https://github.com/torvalds/linux/blob/master/include/uapi/asm-generic/ioctl.h#L85-L91
> > >> and let's VFS layer do the check for each driver.
> > >>
> > >> On Fri, Jan 28, 2022 at 5:14 PM Petro Karashchenko <
> > >> petro.karashche...@gmail.com> wrote:
> > >>
> > >> > Hello team,
> > >> >
> > >> > Recently I have noticed that there are many places in code where
> > >> > register_driver() is called with non-zero mode with file operation
> > >> > structures that have neither read nor write APIs implemented. For
> > >> > example "ret = register_driver(path, &opamp_fops, 0444, dev);" while
> > >> > opamp_fops has only "opamp_open", "opamp_close" and "opamp_ioctl"
> > >> > implemented. I made a PR to fix it
> > >> > https://github.com/apache/incubator-nuttx/pull/5347 and change mode
> > >> > from "0444" to "", but want to ask if anyone sees any drawback
> in
> > >> > such an approach? Maybe I'm missing something?
> > >> >
> > >> > Best regards,
> > >> > Petro
> > >> >
> > >
>


Re: [VOTE] Apache NuttX 10.3.0 (incubating) RC0 release

2022-04-01 Thread Abdelatif Guettouche
+1

I checked:
1. NOTICE, DISCLAIMER and LICENSE files exist.
2. Incubating in name
3. Can build from source.

Thank you Alin!

On Wed, Mar 30, 2022 at 6:50 PM Alin Jerpelea  wrote:
>
> Hello all,
>
>
> Apache NuttX (Incubating) 10.3.0 RC0 has been staged under [1] and it's
> time to vote on accepting it for release. If approved we will seek
> final release approval from the IPMC. Voting will be open for 72hr.
>
> A minimum of 3 binding +1 votes and more binding +1 than binding -1 are
> required to pass.
>
> The Apache requirements for approving a release can be found here [3]
> "Before voting +1 [P]PMC members are required to download the signed
> source code package, compile it as provided, and test the resulting
> executable on their own platform, along with also verifying that the
> package meets the requirements of the ASF policy on releases."
>
> A document to walk through some of this process has been published on
> our project wiki and can be found here [4].
>
> [ ] +1 accept (indicate what you validated - e.g. performed the non-RM
> items in [4])
> [ ] -1 reject (explanation required)
>
> Thank you all,
> Alin Jerpelea
>
> SCM Information:
>   Release tag: nuttx-10.3.0-RC0
>   Hash for the release incubating-nuttx tag:
> c18075779548fcba3f2978689888af8c3b9c959c
>   Hash for the release incubating-nuttx-apps tag:
> 4a2aa6d8cffb6eef45d445ca42a3653f700a2565
>
> [1] https://dist.apache.org/repos/dist/dev/incubator/nuttx/10.3.0-RC0/
> [2]
> https://raw.githubusercontent.com/apache/incubator-nuttx/nuttx-10.3.0-RC0/ReleaseNotes
> [3] https://www.apache.org/dev/release.html#approving-a-release
> [4]
> https://cwiki.apache.org/confluence/display/NUTTX/Validating+a+staged+Release


Re: Article: ST7789 Display with LVGL Graphics

2022-04-01 Thread Tomasz CEDRO
cool! congratz! :-)

--
CeDeROM, SQ7MHZ, http://www.tomek.cedro.info


Re: register_driver with 0000 mode

2022-04-01 Thread Alan Carvalho de Assis
Hi Petro,

I saw your PR #1117 but I think opening a device file with flag 0 is
not correct, please see the open man-pages:

alan@dev:/tmp$ man 2 open

   The argument flags must include one of the  following  access
modes:  O_RDONLY,  O_WRONLY,  or
   O_RDWR.  These request opening the file read-only, write-only,
or read/write, respectively.

Also the opengroup say something similar:

https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html

"Values for oflag are constructed by a bitwise-inclusive OR of flags
from the following list, defined in . Applications shall
specify exactly one of the first five values (file access modes) below
in the value of oflag:"

The man pages uses "MUST", the OpenGroups uses "SHALL", but according
to RFC2119 they are equivalents:

https://www.ietf.org/rfc/rfc2119.txt

BR,

Alan

On 4/1/22, Petro Karashchenko  wrote:
> Hi,
>
> I want to resume this thread again because after reexamined code carefully
> I found that VFS layer has an API
>
> int inode_checkflags(FAR struct inode *inode, int oflags)
> {
>   if (((oflags & O_RDOK) != 0 && !inode->u.i_ops->read) ||
>   ((oflags & O_WROK) != 0 && !inode->u.i_ops->write))
> {
>   return -EACCES;
> }
>   else
> {
>   return OK;
> }
> }
>
> That checks if read and write handlers are available, so all our discussion
> about R/W mode for IOCTL does not make any sense. We either need to remove
> this check or register VFS nodes with proper permissions and open files
> with correct flags. So if the driver does not have neither read nor write
> handlers the "" mode should be used and "0" should be used during
> opening of a file. Or we need to remove "inode_checkflags()".
>
> Best regards,
> Petro
>
> пт, 28 січ. 2022 р. о 15:11 Petro Karashchenko
> 
> пише:
>
>> I see. Thank you for the feedback. I will rework changes to get back
>> read permissions.
>>
>> Best regards,
>> Petro
>>
>> пт, 28 січ. 2022 р. о 14:41 Alan Carvalho de Assis 
>> пише:
>> >
>> > Hi Petro,
>> >
>> > The read permission is needed even when you just want to open a file:
>> >
>> > $ vim noreadfile
>> >
>> > $ chmod  noreadfile
>> >
>> > $ ls -l noreadfile
>> > -- 1 user user 5 jan 28 09:24 noreadfile
>> >
>> > $ cat noreadfile
>> > cat: noreadfile: Permission denied
>> >
>> > You can even try to create a C program just to open it, and it will
>> > fail.
>> >
>> > See the man page of open function:
>> >
>> >The argument flags *must* include one of the  following  access
>> >  modes:  O_RDONLY,  O_WRONLY,  or
>> >O_RDWR.  These request opening the file read-only, write-only,
>> > or read/write, respectively.
>> >
>> > This man page makes it clear you must include an access mode, but I
>> > passed 0 to the access mode flag of open() and it was accepted, but
>> > when the file has permission  it returns -EPERM: "Failed to open
>> > file: error -1"
>> >
>> > BR,
>> >
>> > Alan
>> >
>> > On 1/28/22, Petro Karashchenko  wrote:
>> > > Hello,
>> > >
>> > > Yes, but how does this relate to "" mode for "register_driver()"?
>> > > Maybe you can describe some use case so it will become more clear?
>> > > Currently ioctl works fine if driver is registered with ""
>> permission
>> > > mode.
>> > >
>> > > Best regards,
>> > > Petro
>> > >
>> > > пт, 28 січ. 2022 р. о 11:39 Xiang Xiao 
>> пише:
>> > >>
>> > >> If we want to do the correct permission check, the ioctl handler
>> needs to
>> > >> check R/W bit by itself based on how the ioctl is implemented.
>> > >> Or follow up how Linux encode the needed permission into each IOCTL:
>> > >>
>> https://github.com/torvalds/linux/blob/master/include/uapi/asm-generic/ioctl.h#L85-L91
>> > >> and let's VFS layer do the check for each driver.
>> > >>
>> > >> On Fri, Jan 28, 2022 at 5:14 PM Petro Karashchenko <
>> > >> petro.karashche...@gmail.com> wrote:
>> > >>
>> > >> > Hello team,
>> > >> >
>> > >> > Recently I have noticed that there are many places in code where
>> > >> > register_driver() is called with non-zero mode with file operation
>> > >> > structures that have neither read nor write APIs implemented. For
>> > >> > example "ret = register_driver(path, &opamp_fops, 0444, dev);"
>> > >> > while
>> > >> > opamp_fops has only "opamp_open", "opamp_close" and "opamp_ioctl"
>> > >> > implemented. I made a PR to fix it
>> > >> > https://github.com/apache/incubator-nuttx/pull/5347 and change
>> > >> > mode
>> > >> > from "0444" to "", but want to ask if anyone sees any drawback
>> in
>> > >> > such an approach? Maybe I'm missing something?
>> > >> >
>> > >> > Best regards,
>> > >> > Petro
>> > >> >
>> > >
>>
>


Re: register_driver with 0000 mode

2022-04-01 Thread Petro Karashchenko
So Alan do you suggest to remove inode_checkflags?

On Fri, Apr 1, 2022, 4:13 PM Alan Carvalho de Assis 
wrote:

> Hi Petro,
>
> I saw your PR #1117 but I think opening a device file with flag 0 is
> not correct, please see the open man-pages:
>
> alan@dev:/tmp$ man 2 open
>
>The argument flags must include one of the  following  access
> modes:  O_RDONLY,  O_WRONLY,  or
>O_RDWR.  These request opening the file read-only, write-only,
> or read/write, respectively.
>
> Also the opengroup say something similar:
>
> https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html
>
> "Values for oflag are constructed by a bitwise-inclusive OR of flags
> from the following list, defined in . Applications shall
> specify exactly one of the first five values (file access modes) below
> in the value of oflag:"
>
> The man pages uses "MUST", the OpenGroups uses "SHALL", but according
> to RFC2119 they are equivalents:
>
> https://www.ietf.org/rfc/rfc2119.txt
>
> BR,
>
> Alan
>
> On 4/1/22, Petro Karashchenko  wrote:
> > Hi,
> >
> > I want to resume this thread again because after reexamined code
> carefully
> > I found that VFS layer has an API
> >
> > int inode_checkflags(FAR struct inode *inode, int oflags)
> > {
> >   if (((oflags & O_RDOK) != 0 && !inode->u.i_ops->read) ||
> >   ((oflags & O_WROK) != 0 && !inode->u.i_ops->write))
> > {
> >   return -EACCES;
> > }
> >   else
> > {
> >   return OK;
> > }
> > }
> >
> > That checks if read and write handlers are available, so all our
> discussion
> > about R/W mode for IOCTL does not make any sense. We either need to
> remove
> > this check or register VFS nodes with proper permissions and open files
> > with correct flags. So if the driver does not have neither read nor write
> > handlers the "" mode should be used and "0" should be used during
> > opening of a file. Or we need to remove "inode_checkflags()".
> >
> > Best regards,
> > Petro
> >
> > пт, 28 січ. 2022 р. о 15:11 Petro Karashchenko
> > 
> > пише:
> >
> >> I see. Thank you for the feedback. I will rework changes to get back
> >> read permissions.
> >>
> >> Best regards,
> >> Petro
> >>
> >> пт, 28 січ. 2022 р. о 14:41 Alan Carvalho de Assis 
> >> пише:
> >> >
> >> > Hi Petro,
> >> >
> >> > The read permission is needed even when you just want to open a file:
> >> >
> >> > $ vim noreadfile
> >> >
> >> > $ chmod  noreadfile
> >> >
> >> > $ ls -l noreadfile
> >> > -- 1 user user 5 jan 28 09:24 noreadfile
> >> >
> >> > $ cat noreadfile
> >> > cat: noreadfile: Permission denied
> >> >
> >> > You can even try to create a C program just to open it, and it will
> >> > fail.
> >> >
> >> > See the man page of open function:
> >> >
> >> >The argument flags *must* include one of the  following  access
> >> >  modes:  O_RDONLY,  O_WRONLY,  or
> >> >O_RDWR.  These request opening the file read-only, write-only,
> >> > or read/write, respectively.
> >> >
> >> > This man page makes it clear you must include an access mode, but I
> >> > passed 0 to the access mode flag of open() and it was accepted, but
> >> > when the file has permission  it returns -EPERM: "Failed to open
> >> > file: error -1"
> >> >
> >> > BR,
> >> >
> >> > Alan
> >> >
> >> > On 1/28/22, Petro Karashchenko  wrote:
> >> > > Hello,
> >> > >
> >> > > Yes, but how does this relate to "" mode for
> "register_driver()"?
> >> > > Maybe you can describe some use case so it will become more clear?
> >> > > Currently ioctl works fine if driver is registered with ""
> >> permission
> >> > > mode.
> >> > >
> >> > > Best regards,
> >> > > Petro
> >> > >
> >> > > пт, 28 січ. 2022 р. о 11:39 Xiang Xiao 
> >> пише:
> >> > >>
> >> > >> If we want to do the correct permission check, the ioctl handler
> >> needs to
> >> > >> check R/W bit by itself based on how the ioctl is implemented.
> >> > >> Or follow up how Linux encode the needed permission into each
> IOCTL:
> >> > >>
> >>
> https://github.com/torvalds/linux/blob/master/include/uapi/asm-generic/ioctl.h#L85-L91
> >> > >> and let's VFS layer do the check for each driver.
> >> > >>
> >> > >> On Fri, Jan 28, 2022 at 5:14 PM Petro Karashchenko <
> >> > >> petro.karashche...@gmail.com> wrote:
> >> > >>
> >> > >> > Hello team,
> >> > >> >
> >> > >> > Recently I have noticed that there are many places in code where
> >> > >> > register_driver() is called with non-zero mode with file
> operation
> >> > >> > structures that have neither read nor write APIs implemented. For
> >> > >> > example "ret = register_driver(path, &opamp_fops, 0444, dev);"
> >> > >> > while
> >> > >> > opamp_fops has only "opamp_open", "opamp_close" and "opamp_ioctl"
> >> > >> > implemented. I made a PR to fix it
> >> > >> > https://github.com/apache/incubator-nuttx/pull/5347 and change
> >> > >> > mode
> >> > >> > from "0444" to "", but want to ask if anyone sees any
> drawback
> >> in
> >> > >> > such an approach? Maybe I'm missing something?
> >> 

Re: register_driver with 0000 mode

2022-04-01 Thread Xiang Xiao
It's better to check ioctl callback too since ioctl means the driver has
the compatibility of read(i)and write(o).

On Fri, Apr 1, 2022 at 9:15 PM Petro Karashchenko <
petro.karashche...@gmail.com> wrote:

> So Alan do you suggest to remove inode_checkflags?
>
> On Fri, Apr 1, 2022, 4:13 PM Alan Carvalho de Assis 
> wrote:
>
> > Hi Petro,
> >
> > I saw your PR #1117 but I think opening a device file with flag 0 is
> > not correct, please see the open man-pages:
> >
> > alan@dev:/tmp$ man 2 open
> >
> >The argument flags must include one of the  following  access
> > modes:  O_RDONLY,  O_WRONLY,  or
> >O_RDWR.  These request opening the file read-only, write-only,
> > or read/write, respectively.
> >
> > Also the opengroup say something similar:
> >
> > https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html
> >
> > "Values for oflag are constructed by a bitwise-inclusive OR of flags
> > from the following list, defined in . Applications shall
> > specify exactly one of the first five values (file access modes) below
> > in the value of oflag:"
> >
> > The man pages uses "MUST", the OpenGroups uses "SHALL", but according
> > to RFC2119 they are equivalents:
> >
> > https://www.ietf.org/rfc/rfc2119.txt
> >
> > BR,
> >
> > Alan
> >
> > On 4/1/22, Petro Karashchenko  wrote:
> > > Hi,
> > >
> > > I want to resume this thread again because after reexamined code
> > carefully
> > > I found that VFS layer has an API
> > >
> > > int inode_checkflags(FAR struct inode *inode, int oflags)
> > > {
> > >   if (((oflags & O_RDOK) != 0 && !inode->u.i_ops->read) ||
> > >   ((oflags & O_WROK) != 0 && !inode->u.i_ops->write))
> > > {
> > >   return -EACCES;
> > > }
> > >   else
> > > {
> > >   return OK;
> > > }
> > > }
> > >
> > > That checks if read and write handlers are available, so all our
> > discussion
> > > about R/W mode for IOCTL does not make any sense. We either need to
> > remove
> > > this check or register VFS nodes with proper permissions and open files
> > > with correct flags. So if the driver does not have neither read nor
> write
> > > handlers the "" mode should be used and "0" should be used during
> > > opening of a file. Or we need to remove "inode_checkflags()".
> > >
> > > Best regards,
> > > Petro
> > >
> > > пт, 28 січ. 2022 р. о 15:11 Petro Karashchenko
> > > 
> > > пише:
> > >
> > >> I see. Thank you for the feedback. I will rework changes to get back
> > >> read permissions.
> > >>
> > >> Best regards,
> > >> Petro
> > >>
> > >> пт, 28 січ. 2022 р. о 14:41 Alan Carvalho de Assis  >
> > >> пише:
> > >> >
> > >> > Hi Petro,
> > >> >
> > >> > The read permission is needed even when you just want to open a
> file:
> > >> >
> > >> > $ vim noreadfile
> > >> >
> > >> > $ chmod  noreadfile
> > >> >
> > >> > $ ls -l noreadfile
> > >> > -- 1 user user 5 jan 28 09:24 noreadfile
> > >> >
> > >> > $ cat noreadfile
> > >> > cat: noreadfile: Permission denied
> > >> >
> > >> > You can even try to create a C program just to open it, and it will
> > >> > fail.
> > >> >
> > >> > See the man page of open function:
> > >> >
> > >> >The argument flags *must* include one of the  following
> access
> > >> >  modes:  O_RDONLY,  O_WRONLY,  or
> > >> >O_RDWR.  These request opening the file read-only,
> write-only,
> > >> > or read/write, respectively.
> > >> >
> > >> > This man page makes it clear you must include an access mode, but I
> > >> > passed 0 to the access mode flag of open() and it was accepted, but
> > >> > when the file has permission  it returns -EPERM: "Failed to open
> > >> > file: error -1"
> > >> >
> > >> > BR,
> > >> >
> > >> > Alan
> > >> >
> > >> > On 1/28/22, Petro Karashchenko 
> wrote:
> > >> > > Hello,
> > >> > >
> > >> > > Yes, but how does this relate to "" mode for
> > "register_driver()"?
> > >> > > Maybe you can describe some use case so it will become more clear?
> > >> > > Currently ioctl works fine if driver is registered with ""
> > >> permission
> > >> > > mode.
> > >> > >
> > >> > > Best regards,
> > >> > > Petro
> > >> > >
> > >> > > пт, 28 січ. 2022 р. о 11:39 Xiang Xiao  >
> > >> пише:
> > >> > >>
> > >> > >> If we want to do the correct permission check, the ioctl handler
> > >> needs to
> > >> > >> check R/W bit by itself based on how the ioctl is implemented.
> > >> > >> Or follow up how Linux encode the needed permission into each
> > IOCTL:
> > >> > >>
> > >>
> >
> https://github.com/torvalds/linux/blob/master/include/uapi/asm-generic/ioctl.h#L85-L91
> > >> > >> and let's VFS layer do the check for each driver.
> > >> > >>
> > >> > >> On Fri, Jan 28, 2022 at 5:14 PM Petro Karashchenko <
> > >> > >> petro.karashche...@gmail.com> wrote:
> > >> > >>
> > >> > >> > Hello team,
> > >> > >> >
> > >> > >> > Recently I have noticed that there are many places in code
> where
> > >> > >> > register_driver() is called with non-zero mode with file
> > operation
> > >> > >> > structures t

Re: register_driver with 0000 mode

2022-04-01 Thread Alan Carvalho de Assis
I think the device file shouldn't be created with permission 000.

Look inside your Linux /dev all device files have RW permission for
root, some give only R for group and others.

So, probably we need to fix the device register creation, not removing
the flag check.

BR,

Alan

On 4/1/22, Xiang Xiao  wrote:
> It's better to check ioctl callback too since ioctl means the driver has
> the compatibility of read(i)and write(o).
>
> On Fri, Apr 1, 2022 at 9:15 PM Petro Karashchenko <
> petro.karashche...@gmail.com> wrote:
>
>> So Alan do you suggest to remove inode_checkflags?
>>
>> On Fri, Apr 1, 2022, 4:13 PM Alan Carvalho de Assis 
>> wrote:
>>
>> > Hi Petro,
>> >
>> > I saw your PR #1117 but I think opening a device file with flag 0 is
>> > not correct, please see the open man-pages:
>> >
>> > alan@dev:/tmp$ man 2 open
>> >
>> >The argument flags must include one of the  following  access
>> > modes:  O_RDONLY,  O_WRONLY,  or
>> >O_RDWR.  These request opening the file read-only, write-only,
>> > or read/write, respectively.
>> >
>> > Also the opengroup say something similar:
>> >
>> > https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html
>> >
>> > "Values for oflag are constructed by a bitwise-inclusive OR of flags
>> > from the following list, defined in . Applications shall
>> > specify exactly one of the first five values (file access modes) below
>> > in the value of oflag:"
>> >
>> > The man pages uses "MUST", the OpenGroups uses "SHALL", but according
>> > to RFC2119 they are equivalents:
>> >
>> > https://www.ietf.org/rfc/rfc2119.txt
>> >
>> > BR,
>> >
>> > Alan
>> >
>> > On 4/1/22, Petro Karashchenko  wrote:
>> > > Hi,
>> > >
>> > > I want to resume this thread again because after reexamined code
>> > carefully
>> > > I found that VFS layer has an API
>> > >
>> > > int inode_checkflags(FAR struct inode *inode, int oflags)
>> > > {
>> > >   if (((oflags & O_RDOK) != 0 && !inode->u.i_ops->read) ||
>> > >   ((oflags & O_WROK) != 0 && !inode->u.i_ops->write))
>> > > {
>> > >   return -EACCES;
>> > > }
>> > >   else
>> > > {
>> > >   return OK;
>> > > }
>> > > }
>> > >
>> > > That checks if read and write handlers are available, so all our
>> > discussion
>> > > about R/W mode for IOCTL does not make any sense. We either need to
>> > remove
>> > > this check or register VFS nodes with proper permissions and open
>> > > files
>> > > with correct flags. So if the driver does not have neither read nor
>> write
>> > > handlers the "" mode should be used and "0" should be used during
>> > > opening of a file. Or we need to remove "inode_checkflags()".
>> > >
>> > > Best regards,
>> > > Petro
>> > >
>> > > пт, 28 січ. 2022 р. о 15:11 Petro Karashchenko
>> > > 
>> > > пише:
>> > >
>> > >> I see. Thank you for the feedback. I will rework changes to get back
>> > >> read permissions.
>> > >>
>> > >> Best regards,
>> > >> Petro
>> > >>
>> > >> пт, 28 січ. 2022 р. о 14:41 Alan Carvalho de Assis
>> > >> > >
>> > >> пише:
>> > >> >
>> > >> > Hi Petro,
>> > >> >
>> > >> > The read permission is needed even when you just want to open a
>> file:
>> > >> >
>> > >> > $ vim noreadfile
>> > >> >
>> > >> > $ chmod  noreadfile
>> > >> >
>> > >> > $ ls -l noreadfile
>> > >> > -- 1 user user 5 jan 28 09:24 noreadfile
>> > >> >
>> > >> > $ cat noreadfile
>> > >> > cat: noreadfile: Permission denied
>> > >> >
>> > >> > You can even try to create a C program just to open it, and it
>> > >> > will
>> > >> > fail.
>> > >> >
>> > >> > See the man page of open function:
>> > >> >
>> > >> >The argument flags *must* include one of the  following
>> access
>> > >> >  modes:  O_RDONLY,  O_WRONLY,  or
>> > >> >O_RDWR.  These request opening the file read-only,
>> write-only,
>> > >> > or read/write, respectively.
>> > >> >
>> > >> > This man page makes it clear you must include an access mode, but
>> > >> > I
>> > >> > passed 0 to the access mode flag of open() and it was accepted,
>> > >> > but
>> > >> > when the file has permission  it returns -EPERM: "Failed to
>> > >> > open
>> > >> > file: error -1"
>> > >> >
>> > >> > BR,
>> > >> >
>> > >> > Alan
>> > >> >
>> > >> > On 1/28/22, Petro Karashchenko 
>> wrote:
>> > >> > > Hello,
>> > >> > >
>> > >> > > Yes, but how does this relate to "" mode for
>> > "register_driver()"?
>> > >> > > Maybe you can describe some use case so it will become more
>> > >> > > clear?
>> > >> > > Currently ioctl works fine if driver is registered with ""
>> > >> permission
>> > >> > > mode.
>> > >> > >
>> > >> > > Best regards,
>> > >> > > Petro
>> > >> > >
>> > >> > > пт, 28 січ. 2022 р. о 11:39 Xiang Xiao
>> > >> > > > >
>> > >> пише:
>> > >> > >>
>> > >> > >> If we want to do the correct permission check, the ioctl
>> > >> > >> handler
>> > >> needs to
>> > >> > >> check R/W bit by itself based on how the ioctl is implemented.
>> > >> > >> Or follow up how Linux encode the needed permission into each
>> 

Re: Article: ST7789 Display with LVGL Graphics

2022-04-01 Thread Alan Carvalho de Assis
Hi Lup,

Nice work! Kudos!!!

Did you manually change the Red to Blue color? If you are running the
default LVGL 7.3 demo the color should be Red see:

ESP32 with NuttX:
https://www.youtube.com/watch?v=b2m4LNd29ao

ESP32 with Arduino:
https://www.youtube.com/watch?v=Zq_MZXYbdgI

Maybe your R and B in your RGB are inverted.

BR,

Alan

On 4/1/22, Lee, Lup Yuen  wrote:
> This article explains how I configured the Sitronix ST7789 SPI Display and
> LVGL Graphics Library for NuttX:
>
> https://lupyuen.github.io/articles/st7789
>
> Lup
>


Re: [VOTE] Apache NuttX 10.3.0 (incubating) RC0 release

2022-04-01 Thread Nathan Hartman
On Wed, Mar 30, 2022 at 12:50 PM Alin Jerpelea  wrote:
>
> Hello all,
>
>
> Apache NuttX (Incubating) 10.3.0 RC0 has been staged under [1] and it's
> time to vote on accepting it for release. If approved we will seek
> final release approval from the IPMC. Voting will be open for 72hr.
>
> A minimum of 3 binding +1 votes and more binding +1 than binding -1 are
> required to pass.
>
> The Apache requirements for approving a release can be found here [3]
> "Before voting +1 [P]PMC members are required to download the signed
> source code package, compile it as provided, and test the resulting
> executable on their own platform, along with also verifying that the
> package meets the requirements of the ASF policy on releases."
>
> A document to walk through some of this process has been published on
> our project wiki and can be found here [4].
>
> [ ] +1 accept (indicate what you validated - e.g. performed the non-RM
> items in [4])
> [ ] -1 reject (explanation required)
>
> Thank you all,
> Alin Jerpelea
>
> SCM Information:
>   Release tag: nuttx-10.3.0-RC0
>   Hash for the release incubating-nuttx tag:
> c18075779548fcba3f2978689888af8c3b9c959c
>   Hash for the release incubating-nuttx-apps tag:
> 4a2aa6d8cffb6eef45d445ca42a3653f700a2565
>
> [1] https://dist.apache.org/repos/dist/dev/incubator/nuttx/10.3.0-RC0/
> [2]
> https://raw.githubusercontent.com/apache/incubator-nuttx/nuttx-10.3.0-RC0/ReleaseNotes
> [3] https://www.apache.org/dev/release.html#approving-a-release
> [4]
> https://cwiki.apache.org/confluence/display/NUTTX/Validating+a+staged+Release


Summary:
+1 to release (binding)

Per Alan's request for size information [1]:

* NuttX-10.3.0-RC0, b-g474e-dpow1:nsh configuration:

$ arm-none-eabi-size nuttx
   textdata bss dec hex filename
 117851 6362256  120743   1d7a7 nuttx

* For comparison, same configuration on NuttX-10.2.0:

$ arm-none-eabi-size nuttx
   textdata bss dec hex filename
 115331 6242500  118455   1ceb7 nuttx

Text increases 2520, data increases 12, bss decreases 244.

tl;dr: This release has gotten bigger compared to last 2 releases.

Development system: Linux
(Debian 4.19.0-20-rt-amd64 x86_64)

Verified:
* Signatures
* SHA-512 sums
* Incubating in artifact names
* LICENSE, NOTICE, README.md present in both tarballs
* DISCLAIMER-WIP is removed as licenses have been migrated to Apache
  2.0 or documented in LICENSE
* Build and run b-g474e-dpow1:nsh configuration successfully
* Release notes have been staged

Dependencies:
* gcc-arm-none-eabi-7-2017-q4-major
* kconfig-conf from NuttX tools repository

Other dependencies from Debian packages:
* binutils-dev 2.31.1-16
* bison 2:3.3.2.dfsg-1
* flex 2.6.4-6.2
* gperf 3.1-1
* libelf-dev 0.176-1.1
* libgmp-dev 2:6.1.2+dfsg-4
* libisl-dev 0.20-2
* libmpc-dev 1.1.0-1
* libmpfr-dev 4.0.2-1
* libncurses5-dev 6.1+20181013-2+deb10u2
* libusb-1.0-0-dev 2:1.0.22-2
* libusb-dev 2:0.1.12-32
* openocd 0.10.0+dev-01241-gdadf46f6-dirty
* texinfo 6.5.0.dfsg.1-4+b1

Thanks to our RM and to everyone in the Apache NuttX community for
making this release (candidate) possible!

References:

[1] Alan's message to the dev@nuttx.a.o thread "Re: [VOTE] Apache
NuttX 10.0.0 (incubating) RC0 release" on 26 Nov 2020, archived:
https://lists.apache.org/thread/nxvwxol948psr2z7fc6cwtdv9ofoz9yj

Cheers,
Nathan


Re: register_driver with 0000 mode

2022-04-01 Thread Petro Karashchenko
Hello,

Here I'm talking not about driver registration permission, but more about
the "oflag" parameter to "open()" call.

I just tried a quick example on MAC

#include 
#include 

int main(void)
{
  int fd = open("test.txt", 0);
  if (fd < 0)
printf("A\n");
  else
printf("B\n");

  return 0;
}

The "B" is printed if the file exists. If you know the system that will run
this sample code and will print "A", please let me know.

Best regards,
Petro

пт, 1 квіт. 2022 р. о 16:27 Alan Carvalho de Assis  пише:

> I think the device file shouldn't be created with permission 000.
>
> Look inside your Linux /dev all device files have RW permission for
> root, some give only R for group and others.
>
> So, probably we need to fix the device register creation, not removing
> the flag check.
>
> BR,
>
> Alan
>
> On 4/1/22, Xiang Xiao  wrote:
> > It's better to check ioctl callback too since ioctl means the driver has
> > the compatibility of read(i)and write(o).
> >
> > On Fri, Apr 1, 2022 at 9:15 PM Petro Karashchenko <
> > petro.karashche...@gmail.com> wrote:
> >
> >> So Alan do you suggest to remove inode_checkflags?
> >>
> >> On Fri, Apr 1, 2022, 4:13 PM Alan Carvalho de Assis 
> >> wrote:
> >>
> >> > Hi Petro,
> >> >
> >> > I saw your PR #1117 but I think opening a device file with flag 0 is
> >> > not correct, please see the open man-pages:
> >> >
> >> > alan@dev:/tmp$ man 2 open
> >> >
> >> >The argument flags must include one of the  following  access
> >> > modes:  O_RDONLY,  O_WRONLY,  or
> >> >O_RDWR.  These request opening the file read-only, write-only,
> >> > or read/write, respectively.
> >> >
> >> > Also the opengroup say something similar:
> >> >
> >> > https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html
> >> >
> >> > "Values for oflag are constructed by a bitwise-inclusive OR of flags
> >> > from the following list, defined in . Applications shall
> >> > specify exactly one of the first five values (file access modes) below
> >> > in the value of oflag:"
> >> >
> >> > The man pages uses "MUST", the OpenGroups uses "SHALL", but according
> >> > to RFC2119 they are equivalents:
> >> >
> >> > https://www.ietf.org/rfc/rfc2119.txt
> >> >
> >> > BR,
> >> >
> >> > Alan
> >> >
> >> > On 4/1/22, Petro Karashchenko  wrote:
> >> > > Hi,
> >> > >
> >> > > I want to resume this thread again because after reexamined code
> >> > carefully
> >> > > I found that VFS layer has an API
> >> > >
> >> > > int inode_checkflags(FAR struct inode *inode, int oflags)
> >> > > {
> >> > >   if (((oflags & O_RDOK) != 0 && !inode->u.i_ops->read) ||
> >> > >   ((oflags & O_WROK) != 0 && !inode->u.i_ops->write))
> >> > > {
> >> > >   return -EACCES;
> >> > > }
> >> > >   else
> >> > > {
> >> > >   return OK;
> >> > > }
> >> > > }
> >> > >
> >> > > That checks if read and write handlers are available, so all our
> >> > discussion
> >> > > about R/W mode for IOCTL does not make any sense. We either need to
> >> > remove
> >> > > this check or register VFS nodes with proper permissions and open
> >> > > files
> >> > > with correct flags. So if the driver does not have neither read nor
> >> write
> >> > > handlers the "" mode should be used and "0" should be used
> during
> >> > > opening of a file. Or we need to remove "inode_checkflags()".
> >> > >
> >> > > Best regards,
> >> > > Petro
> >> > >
> >> > > пт, 28 січ. 2022 р. о 15:11 Petro Karashchenko
> >> > > 
> >> > > пише:
> >> > >
> >> > >> I see. Thank you for the feedback. I will rework changes to get
> back
> >> > >> read permissions.
> >> > >>
> >> > >> Best regards,
> >> > >> Petro
> >> > >>
> >> > >> пт, 28 січ. 2022 р. о 14:41 Alan Carvalho de Assis
> >> > >>  >> >
> >> > >> пише:
> >> > >> >
> >> > >> > Hi Petro,
> >> > >> >
> >> > >> > The read permission is needed even when you just want to open a
> >> file:
> >> > >> >
> >> > >> > $ vim noreadfile
> >> > >> >
> >> > >> > $ chmod  noreadfile
> >> > >> >
> >> > >> > $ ls -l noreadfile
> >> > >> > -- 1 user user 5 jan 28 09:24 noreadfile
> >> > >> >
> >> > >> > $ cat noreadfile
> >> > >> > cat: noreadfile: Permission denied
> >> > >> >
> >> > >> > You can even try to create a C program just to open it, and it
> >> > >> > will
> >> > >> > fail.
> >> > >> >
> >> > >> > See the man page of open function:
> >> > >> >
> >> > >> >The argument flags *must* include one of the  following
> >> access
> >> > >> >  modes:  O_RDONLY,  O_WRONLY,  or
> >> > >> >O_RDWR.  These request opening the file read-only,
> >> write-only,
> >> > >> > or read/write, respectively.
> >> > >> >
> >> > >> > This man page makes it clear you must include an access mode, but
> >> > >> > I
> >> > >> > passed 0 to the access mode flag of open() and it was accepted,
> >> > >> > but
> >> > >> > when the file has permission  it returns -EPERM: "Failed to
> >> > >> > open
> >> > >> > file: error -1"
> >> > >> >
> >> > >> > BR,
> >> > >> >
> >> > >> > Alan
> >> 

Re: register_driver with 0000 mode

2022-04-01 Thread Xiang Xiao
We need to try some ioctl with read/write some driver(e.g. serial driver
baud) internal state.


On Fri, Apr 1, 2022 at 10:05 PM Petro Karashchenko <
petro.karashche...@gmail.com> wrote:

> Hello,
>
> Here I'm talking not about driver registration permission, but more about
> the "oflag" parameter to "open()" call.
>
> I just tried a quick example on MAC
>
> #include 
> #include 
>
> int main(void)
> {
>   int fd = open("test.txt", 0);
>   if (fd < 0)
> printf("A\n");
>   else
> printf("B\n");
>
>   return 0;
> }
>
> The "B" is printed if the file exists. If you know the system that will run
> this sample code and will print "A", please let me know.
>
> Best regards,
> Petro
>
> пт, 1 квіт. 2022 р. о 16:27 Alan Carvalho de Assis 
> пише:
>
> > I think the device file shouldn't be created with permission 000.
> >
> > Look inside your Linux /dev all device files have RW permission for
> > root, some give only R for group and others.
> >
> > So, probably we need to fix the device register creation, not removing
> > the flag check.
> >
> > BR,
> >
> > Alan
> >
> > On 4/1/22, Xiang Xiao  wrote:
> > > It's better to check ioctl callback too since ioctl means the driver
> has
> > > the compatibility of read(i)and write(o).
> > >
> > > On Fri, Apr 1, 2022 at 9:15 PM Petro Karashchenko <
> > > petro.karashche...@gmail.com> wrote:
> > >
> > >> So Alan do you suggest to remove inode_checkflags?
> > >>
> > >> On Fri, Apr 1, 2022, 4:13 PM Alan Carvalho de Assis <
> acas...@gmail.com>
> > >> wrote:
> > >>
> > >> > Hi Petro,
> > >> >
> > >> > I saw your PR #1117 but I think opening a device file with flag 0 is
> > >> > not correct, please see the open man-pages:
> > >> >
> > >> > alan@dev:/tmp$ man 2 open
> > >> >
> > >> >The argument flags must include one of the  following  access
> > >> > modes:  O_RDONLY,  O_WRONLY,  or
> > >> >O_RDWR.  These request opening the file read-only,
> write-only,
> > >> > or read/write, respectively.
> > >> >
> > >> > Also the opengroup say something similar:
> > >> >
> > >> >
> https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html
> > >> >
> > >> > "Values for oflag are constructed by a bitwise-inclusive OR of flags
> > >> > from the following list, defined in . Applications shall
> > >> > specify exactly one of the first five values (file access modes)
> below
> > >> > in the value of oflag:"
> > >> >
> > >> > The man pages uses "MUST", the OpenGroups uses "SHALL", but
> according
> > >> > to RFC2119 they are equivalents:
> > >> >
> > >> > https://www.ietf.org/rfc/rfc2119.txt
> > >> >
> > >> > BR,
> > >> >
> > >> > Alan
> > >> >
> > >> > On 4/1/22, Petro Karashchenko  wrote:
> > >> > > Hi,
> > >> > >
> > >> > > I want to resume this thread again because after reexamined code
> > >> > carefully
> > >> > > I found that VFS layer has an API
> > >> > >
> > >> > > int inode_checkflags(FAR struct inode *inode, int oflags)
> > >> > > {
> > >> > >   if (((oflags & O_RDOK) != 0 && !inode->u.i_ops->read) ||
> > >> > >   ((oflags & O_WROK) != 0 && !inode->u.i_ops->write))
> > >> > > {
> > >> > >   return -EACCES;
> > >> > > }
> > >> > >   else
> > >> > > {
> > >> > >   return OK;
> > >> > > }
> > >> > > }
> > >> > >
> > >> > > That checks if read and write handlers are available, so all our
> > >> > discussion
> > >> > > about R/W mode for IOCTL does not make any sense. We either need
> to
> > >> > remove
> > >> > > this check or register VFS nodes with proper permissions and open
> > >> > > files
> > >> > > with correct flags. So if the driver does not have neither read
> nor
> > >> write
> > >> > > handlers the "" mode should be used and "0" should be used
> > during
> > >> > > opening of a file. Or we need to remove "inode_checkflags()".
> > >> > >
> > >> > > Best regards,
> > >> > > Petro
> > >> > >
> > >> > > пт, 28 січ. 2022 р. о 15:11 Petro Karashchenko
> > >> > > 
> > >> > > пише:
> > >> > >
> > >> > >> I see. Thank you for the feedback. I will rework changes to get
> > back
> > >> > >> read permissions.
> > >> > >>
> > >> > >> Best regards,
> > >> > >> Petro
> > >> > >>
> > >> > >> пт, 28 січ. 2022 р. о 14:41 Alan Carvalho de Assis
> > >> > >>  > >> >
> > >> > >> пише:
> > >> > >> >
> > >> > >> > Hi Petro,
> > >> > >> >
> > >> > >> > The read permission is needed even when you just want to open a
> > >> file:
> > >> > >> >
> > >> > >> > $ vim noreadfile
> > >> > >> >
> > >> > >> > $ chmod  noreadfile
> > >> > >> >
> > >> > >> > $ ls -l noreadfile
> > >> > >> > -- 1 user user 5 jan 28 09:24 noreadfile
> > >> > >> >
> > >> > >> > $ cat noreadfile
> > >> > >> > cat: noreadfile: Permission denied
> > >> > >> >
> > >> > >> > You can even try to create a C program just to open it, and it
> > >> > >> > will
> > >> > >> > fail.
> > >> > >> >
> > >> > >> > See the man page of open function:
> > >> > >> >
> > >> > >> >The argument flags *must* include one of the  following
> > >> access
> > >> > >> >  modes: 

Re: register_driver with 0000 mode

2022-04-01 Thread Petro Karashchenko
Ok. I will update the code example and come back later with some answers.

On Fri, Apr 1, 2022, 5:32 PM Xiang Xiao  wrote:

> We need to try some ioctl with read/write some driver(e.g. serial driver
> baud) internal state.
>
>
> On Fri, Apr 1, 2022 at 10:05 PM Petro Karashchenko <
> petro.karashche...@gmail.com> wrote:
>
> > Hello,
> >
> > Here I'm talking not about driver registration permission, but more about
> > the "oflag" parameter to "open()" call.
> >
> > I just tried a quick example on MAC
> >
> > #include 
> > #include 
> >
> > int main(void)
> > {
> >   int fd = open("test.txt", 0);
> >   if (fd < 0)
> > printf("A\n");
> >   else
> > printf("B\n");
> >
> >   return 0;
> > }
> >
> > The "B" is printed if the file exists. If you know the system that will
> run
> > this sample code and will print "A", please let me know.
> >
> > Best regards,
> > Petro
> >
> > пт, 1 квіт. 2022 р. о 16:27 Alan Carvalho de Assis 
> > пише:
> >
> > > I think the device file shouldn't be created with permission 000.
> > >
> > > Look inside your Linux /dev all device files have RW permission for
> > > root, some give only R for group and others.
> > >
> > > So, probably we need to fix the device register creation, not removing
> > > the flag check.
> > >
> > > BR,
> > >
> > > Alan
> > >
> > > On 4/1/22, Xiang Xiao  wrote:
> > > > It's better to check ioctl callback too since ioctl means the driver
> > has
> > > > the compatibility of read(i)and write(o).
> > > >
> > > > On Fri, Apr 1, 2022 at 9:15 PM Petro Karashchenko <
> > > > petro.karashche...@gmail.com> wrote:
> > > >
> > > >> So Alan do you suggest to remove inode_checkflags?
> > > >>
> > > >> On Fri, Apr 1, 2022, 4:13 PM Alan Carvalho de Assis <
> > acas...@gmail.com>
> > > >> wrote:
> > > >>
> > > >> > Hi Petro,
> > > >> >
> > > >> > I saw your PR #1117 but I think opening a device file with flag 0
> is
> > > >> > not correct, please see the open man-pages:
> > > >> >
> > > >> > alan@dev:/tmp$ man 2 open
> > > >> >
> > > >> >The argument flags must include one of the  following
> access
> > > >> > modes:  O_RDONLY,  O_WRONLY,  or
> > > >> >O_RDWR.  These request opening the file read-only,
> > write-only,
> > > >> > or read/write, respectively.
> > > >> >
> > > >> > Also the opengroup say something similar:
> > > >> >
> > > >> >
> > https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html
> > > >> >
> > > >> > "Values for oflag are constructed by a bitwise-inclusive OR of
> flags
> > > >> > from the following list, defined in . Applications shall
> > > >> > specify exactly one of the first five values (file access modes)
> > below
> > > >> > in the value of oflag:"
> > > >> >
> > > >> > The man pages uses "MUST", the OpenGroups uses "SHALL", but
> > according
> > > >> > to RFC2119 they are equivalents:
> > > >> >
> > > >> > https://www.ietf.org/rfc/rfc2119.txt
> > > >> >
> > > >> > BR,
> > > >> >
> > > >> > Alan
> > > >> >
> > > >> > On 4/1/22, Petro Karashchenko 
> wrote:
> > > >> > > Hi,
> > > >> > >
> > > >> > > I want to resume this thread again because after reexamined code
> > > >> > carefully
> > > >> > > I found that VFS layer has an API
> > > >> > >
> > > >> > > int inode_checkflags(FAR struct inode *inode, int oflags)
> > > >> > > {
> > > >> > >   if (((oflags & O_RDOK) != 0 && !inode->u.i_ops->read) ||
> > > >> > >   ((oflags & O_WROK) != 0 && !inode->u.i_ops->write))
> > > >> > > {
> > > >> > >   return -EACCES;
> > > >> > > }
> > > >> > >   else
> > > >> > > {
> > > >> > >   return OK;
> > > >> > > }
> > > >> > > }
> > > >> > >
> > > >> > > That checks if read and write handlers are available, so all our
> > > >> > discussion
> > > >> > > about R/W mode for IOCTL does not make any sense. We either need
> > to
> > > >> > remove
> > > >> > > this check or register VFS nodes with proper permissions and
> open
> > > >> > > files
> > > >> > > with correct flags. So if the driver does not have neither read
> > nor
> > > >> write
> > > >> > > handlers the "" mode should be used and "0" should be used
> > > during
> > > >> > > opening of a file. Or we need to remove "inode_checkflags()".
> > > >> > >
> > > >> > > Best regards,
> > > >> > > Petro
> > > >> > >
> > > >> > > пт, 28 січ. 2022 р. о 15:11 Petro Karashchenko
> > > >> > > 
> > > >> > > пише:
> > > >> > >
> > > >> > >> I see. Thank you for the feedback. I will rework changes to get
> > > back
> > > >> > >> read permissions.
> > > >> > >>
> > > >> > >> Best regards,
> > > >> > >> Petro
> > > >> > >>
> > > >> > >> пт, 28 січ. 2022 р. о 14:41 Alan Carvalho de Assis
> > > >> > >>  > > >> >
> > > >> > >> пише:
> > > >> > >> >
> > > >> > >> > Hi Petro,
> > > >> > >> >
> > > >> > >> > The read permission is needed even when you just want to
> open a
> > > >> file:
> > > >> > >> >
> > > >> > >> > $ vim noreadfile
> > > >> > >> >
> > > >> > >> > $ chmod  noreadfile
> > > >> > >> >
> > > >> > >> > $ ls -l noreadfile
> > > >> > >> > ---

Re: register_driver with 0000 mode

2022-04-01 Thread Alan Carvalho de Assis
Hmm, I think oflag equal 0 on Unix(so MacOS)/Linux works because of it:

#define O_RDONLY

So, in fact on Linux/Mac when we are opening a file with oflag 0 we
are opening it for reading only.

On NuttX the value 0 is not defined, O_RDONLY is 1:

#define O_RDONLY(1 << 0)

BR,

Alan

On 4/1/22, Petro Karashchenko  wrote:
> Hello,
>
> Here I'm talking not about driver registration permission, but more about
> the "oflag" parameter to "open()" call.
>
> I just tried a quick example on MAC
>
> #include 
> #include 
>
> int main(void)
> {
>   int fd = open("test.txt", 0);
>   if (fd < 0)
> printf("A\n");
>   else
> printf("B\n");
>
>   return 0;
> }
>
> The "B" is printed if the file exists. If you know the system that will run
> this sample code and will print "A", please let me know.
>
> Best regards,
> Petro
>
> пт, 1 квіт. 2022 р. о 16:27 Alan Carvalho de Assis 
> пише:
>
>> I think the device file shouldn't be created with permission 000.
>>
>> Look inside your Linux /dev all device files have RW permission for
>> root, some give only R for group and others.
>>
>> So, probably we need to fix the device register creation, not removing
>> the flag check.
>>
>> BR,
>>
>> Alan
>>
>> On 4/1/22, Xiang Xiao  wrote:
>> > It's better to check ioctl callback too since ioctl means the driver
>> > has
>> > the compatibility of read(i)and write(o).
>> >
>> > On Fri, Apr 1, 2022 at 9:15 PM Petro Karashchenko <
>> > petro.karashche...@gmail.com> wrote:
>> >
>> >> So Alan do you suggest to remove inode_checkflags?
>> >>
>> >> On Fri, Apr 1, 2022, 4:13 PM Alan Carvalho de Assis
>> >> 
>> >> wrote:
>> >>
>> >> > Hi Petro,
>> >> >
>> >> > I saw your PR #1117 but I think opening a device file with flag 0 is
>> >> > not correct, please see the open man-pages:
>> >> >
>> >> > alan@dev:/tmp$ man 2 open
>> >> >
>> >> >The argument flags must include one of the  following  access
>> >> > modes:  O_RDONLY,  O_WRONLY,  or
>> >> >O_RDWR.  These request opening the file read-only,
>> >> > write-only,
>> >> > or read/write, respectively.
>> >> >
>> >> > Also the opengroup say something similar:
>> >> >
>> >> > https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html
>> >> >
>> >> > "Values for oflag are constructed by a bitwise-inclusive OR of flags
>> >> > from the following list, defined in . Applications shall
>> >> > specify exactly one of the first five values (file access modes)
>> >> > below
>> >> > in the value of oflag:"
>> >> >
>> >> > The man pages uses "MUST", the OpenGroups uses "SHALL", but
>> >> > according
>> >> > to RFC2119 they are equivalents:
>> >> >
>> >> > https://www.ietf.org/rfc/rfc2119.txt
>> >> >
>> >> > BR,
>> >> >
>> >> > Alan
>> >> >
>> >> > On 4/1/22, Petro Karashchenko  wrote:
>> >> > > Hi,
>> >> > >
>> >> > > I want to resume this thread again because after reexamined code
>> >> > carefully
>> >> > > I found that VFS layer has an API
>> >> > >
>> >> > > int inode_checkflags(FAR struct inode *inode, int oflags)
>> >> > > {
>> >> > >   if (((oflags & O_RDOK) != 0 && !inode->u.i_ops->read) ||
>> >> > >   ((oflags & O_WROK) != 0 && !inode->u.i_ops->write))
>> >> > > {
>> >> > >   return -EACCES;
>> >> > > }
>> >> > >   else
>> >> > > {
>> >> > >   return OK;
>> >> > > }
>> >> > > }
>> >> > >
>> >> > > That checks if read and write handlers are available, so all our
>> >> > discussion
>> >> > > about R/W mode for IOCTL does not make any sense. We either need
>> >> > > to
>> >> > remove
>> >> > > this check or register VFS nodes with proper permissions and open
>> >> > > files
>> >> > > with correct flags. So if the driver does not have neither read
>> >> > > nor
>> >> write
>> >> > > handlers the "" mode should be used and "0" should be used
>> during
>> >> > > opening of a file. Or we need to remove "inode_checkflags()".
>> >> > >
>> >> > > Best regards,
>> >> > > Petro
>> >> > >
>> >> > > пт, 28 січ. 2022 р. о 15:11 Petro Karashchenko
>> >> > > 
>> >> > > пише:
>> >> > >
>> >> > >> I see. Thank you for the feedback. I will rework changes to get
>> back
>> >> > >> read permissions.
>> >> > >>
>> >> > >> Best regards,
>> >> > >> Petro
>> >> > >>
>> >> > >> пт, 28 січ. 2022 р. о 14:41 Alan Carvalho de Assis
>> >> > >> > >> >
>> >> > >> пише:
>> >> > >> >
>> >> > >> > Hi Petro,
>> >> > >> >
>> >> > >> > The read permission is needed even when you just want to open a
>> >> file:
>> >> > >> >
>> >> > >> > $ vim noreadfile
>> >> > >> >
>> >> > >> > $ chmod  noreadfile
>> >> > >> >
>> >> > >> > $ ls -l noreadfile
>> >> > >> > -- 1 user user 5 jan 28 09:24 noreadfile
>> >> > >> >
>> >> > >> > $ cat noreadfile
>> >> > >> > cat: noreadfile: Permission denied
>> >> > >> >
>> >> > >> > You can even try to create a C program just to open it, and it
>> >> > >> > will
>> >> > >> > fail.
>> >> > >> >
>> >> > >> > See the man page of open function:
>> >> > >> >
>> >> > >> >The argument flags *must* include one of the  f

Re: register_driver with 0000 mode

2022-04-01 Thread Petro Karashchenko
Yeah,

Probably this is the case. So when it is understood we need to think how to
fix it :)

Best regards,
Petro

пт, 1 квіт. 2022 р. о 17:41 Alan Carvalho de Assis  пише:

> Hmm, I think oflag equal 0 on Unix(so MacOS)/Linux works because of it:
>
> #define O_RDONLY
>
> So, in fact on Linux/Mac when we are opening a file with oflag 0 we
> are opening it for reading only.
>
> On NuttX the value 0 is not defined, O_RDONLY is 1:
>
> #define O_RDONLY(1 << 0)
>
> BR,
>
> Alan
>
> On 4/1/22, Petro Karashchenko  wrote:
> > Hello,
> >
> > Here I'm talking not about driver registration permission, but more about
> > the "oflag" parameter to "open()" call.
> >
> > I just tried a quick example on MAC
> >
> > #include 
> > #include 
> >
> > int main(void)
> > {
> >   int fd = open("test.txt", 0);
> >   if (fd < 0)
> > printf("A\n");
> >   else
> > printf("B\n");
> >
> >   return 0;
> > }
> >
> > The "B" is printed if the file exists. If you know the system that will
> run
> > this sample code and will print "A", please let me know.
> >
> > Best regards,
> > Petro
> >
> > пт, 1 квіт. 2022 р. о 16:27 Alan Carvalho de Assis 
> > пише:
> >
> >> I think the device file shouldn't be created with permission 000.
> >>
> >> Look inside your Linux /dev all device files have RW permission for
> >> root, some give only R for group and others.
> >>
> >> So, probably we need to fix the device register creation, not removing
> >> the flag check.
> >>
> >> BR,
> >>
> >> Alan
> >>
> >> On 4/1/22, Xiang Xiao  wrote:
> >> > It's better to check ioctl callback too since ioctl means the driver
> >> > has
> >> > the compatibility of read(i)and write(o).
> >> >
> >> > On Fri, Apr 1, 2022 at 9:15 PM Petro Karashchenko <
> >> > petro.karashche...@gmail.com> wrote:
> >> >
> >> >> So Alan do you suggest to remove inode_checkflags?
> >> >>
> >> >> On Fri, Apr 1, 2022, 4:13 PM Alan Carvalho de Assis
> >> >> 
> >> >> wrote:
> >> >>
> >> >> > Hi Petro,
> >> >> >
> >> >> > I saw your PR #1117 but I think opening a device file with flag 0
> is
> >> >> > not correct, please see the open man-pages:
> >> >> >
> >> >> > alan@dev:/tmp$ man 2 open
> >> >> >
> >> >> >The argument flags must include one of the  following
> access
> >> >> > modes:  O_RDONLY,  O_WRONLY,  or
> >> >> >O_RDWR.  These request opening the file read-only,
> >> >> > write-only,
> >> >> > or read/write, respectively.
> >> >> >
> >> >> > Also the opengroup say something similar:
> >> >> >
> >> >> >
> https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html
> >> >> >
> >> >> > "Values for oflag are constructed by a bitwise-inclusive OR of
> flags
> >> >> > from the following list, defined in . Applications shall
> >> >> > specify exactly one of the first five values (file access modes)
> >> >> > below
> >> >> > in the value of oflag:"
> >> >> >
> >> >> > The man pages uses "MUST", the OpenGroups uses "SHALL", but
> >> >> > according
> >> >> > to RFC2119 they are equivalents:
> >> >> >
> >> >> > https://www.ietf.org/rfc/rfc2119.txt
> >> >> >
> >> >> > BR,
> >> >> >
> >> >> > Alan
> >> >> >
> >> >> > On 4/1/22, Petro Karashchenko 
> wrote:
> >> >> > > Hi,
> >> >> > >
> >> >> > > I want to resume this thread again because after reexamined code
> >> >> > carefully
> >> >> > > I found that VFS layer has an API
> >> >> > >
> >> >> > > int inode_checkflags(FAR struct inode *inode, int oflags)
> >> >> > > {
> >> >> > >   if (((oflags & O_RDOK) != 0 && !inode->u.i_ops->read) ||
> >> >> > >   ((oflags & O_WROK) != 0 && !inode->u.i_ops->write))
> >> >> > > {
> >> >> > >   return -EACCES;
> >> >> > > }
> >> >> > >   else
> >> >> > > {
> >> >> > >   return OK;
> >> >> > > }
> >> >> > > }
> >> >> > >
> >> >> > > That checks if read and write handlers are available, so all our
> >> >> > discussion
> >> >> > > about R/W mode for IOCTL does not make any sense. We either need
> >> >> > > to
> >> >> > remove
> >> >> > > this check or register VFS nodes with proper permissions and open
> >> >> > > files
> >> >> > > with correct flags. So if the driver does not have neither read
> >> >> > > nor
> >> >> write
> >> >> > > handlers the "" mode should be used and "0" should be used
> >> during
> >> >> > > opening of a file. Or we need to remove "inode_checkflags()".
> >> >> > >
> >> >> > > Best regards,
> >> >> > > Petro
> >> >> > >
> >> >> > > пт, 28 січ. 2022 р. о 15:11 Petro Karashchenko
> >> >> > > 
> >> >> > > пише:
> >> >> > >
> >> >> > >> I see. Thank you for the feedback. I will rework changes to get
> >> back
> >> >> > >> read permissions.
> >> >> > >>
> >> >> > >> Best regards,
> >> >> > >> Petro
> >> >> > >>
> >> >> > >> пт, 28 січ. 2022 р. о 14:41 Alan Carvalho de Assis
> >> >> > >>  >> >> >
> >> >> > >> пише:
> >> >> > >> >
> >> >> > >> > Hi Petro,
> >> >> > >> >
> >> >> > >> > The read permission is needed even when you just want to open
> a
> >> >> file:
> >> >> > >> >
> >> >> > >> > $ vim noreadfile
> >> >>

Re: register_driver with 0000 mode

2022-04-01 Thread Alan Carvalho de Assis
More about it here:
https://stackoverflow.com/questions/61923703/how-to-make-sense-of-o-rdonly-0

So, I agree with the comment that said "calling it flag is misnomer
and misleading"

On Unix/Linux O_RDONLY | O_WRONLY != O_RDWR, on NuttX is it explicitly this way:

#define O_RDWR  (O_RDOK|O_WROK) /* Open for both read & write access */

Now, I'm also confuse about the right thing to do:

1) Fix include/fcntl.h to follow Unix/Linux
2) Keep things we they are and don't accept opening a file if it
doesn't have RDONLY flag (and change all the drivers, event ioctl only
drivers, to include reading flag)
3) Remove the flag checking.

Probably we should do 1) because NuttX follows Unix/Linux approach,
although I agree that Unix/Linux are completely non-sense on this
subject, oflag should be a flag like it is on NuttX :-)

BR,

Alan

On 4/1/22, Alan Carvalho de Assis  wrote:
> Hmm, I think oflag equal 0 on Unix(so MacOS)/Linux works because of it:
>
> #define O_RDONLY  
>
> So, in fact on Linux/Mac when we are opening a file with oflag 0 we
> are opening it for reading only.
>
> On NuttX the value 0 is not defined, O_RDONLY is 1:
>
> #define O_RDONLY(1 << 0)
>
> BR,
>
> Alan
>
> On 4/1/22, Petro Karashchenko  wrote:
>> Hello,
>>
>> Here I'm talking not about driver registration permission, but more about
>> the "oflag" parameter to "open()" call.
>>
>> I just tried a quick example on MAC
>>
>> #include 
>> #include 
>>
>> int main(void)
>> {
>>   int fd = open("test.txt", 0);
>>   if (fd < 0)
>> printf("A\n");
>>   else
>> printf("B\n");
>>
>>   return 0;
>> }
>>
>> The "B" is printed if the file exists. If you know the system that will
>> run
>> this sample code and will print "A", please let me know.
>>
>> Best regards,
>> Petro
>>
>> пт, 1 квіт. 2022 р. о 16:27 Alan Carvalho de Assis 
>> пише:
>>
>>> I think the device file shouldn't be created with permission 000.
>>>
>>> Look inside your Linux /dev all device files have RW permission for
>>> root, some give only R for group and others.
>>>
>>> So, probably we need to fix the device register creation, not removing
>>> the flag check.
>>>
>>> BR,
>>>
>>> Alan
>>>
>>> On 4/1/22, Xiang Xiao  wrote:
>>> > It's better to check ioctl callback too since ioctl means the driver
>>> > has
>>> > the compatibility of read(i)and write(o).
>>> >
>>> > On Fri, Apr 1, 2022 at 9:15 PM Petro Karashchenko <
>>> > petro.karashche...@gmail.com> wrote:
>>> >
>>> >> So Alan do you suggest to remove inode_checkflags?
>>> >>
>>> >> On Fri, Apr 1, 2022, 4:13 PM Alan Carvalho de Assis
>>> >> 
>>> >> wrote:
>>> >>
>>> >> > Hi Petro,
>>> >> >
>>> >> > I saw your PR #1117 but I think opening a device file with flag 0
>>> >> > is
>>> >> > not correct, please see the open man-pages:
>>> >> >
>>> >> > alan@dev:/tmp$ man 2 open
>>> >> >
>>> >> >The argument flags must include one of the  following
>>> >> > access
>>> >> > modes:  O_RDONLY,  O_WRONLY,  or
>>> >> >O_RDWR.  These request opening the file read-only,
>>> >> > write-only,
>>> >> > or read/write, respectively.
>>> >> >
>>> >> > Also the opengroup say something similar:
>>> >> >
>>> >> > https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html
>>> >> >
>>> >> > "Values for oflag are constructed by a bitwise-inclusive OR of
>>> >> > flags
>>> >> > from the following list, defined in . Applications shall
>>> >> > specify exactly one of the first five values (file access modes)
>>> >> > below
>>> >> > in the value of oflag:"
>>> >> >
>>> >> > The man pages uses "MUST", the OpenGroups uses "SHALL", but
>>> >> > according
>>> >> > to RFC2119 they are equivalents:
>>> >> >
>>> >> > https://www.ietf.org/rfc/rfc2119.txt
>>> >> >
>>> >> > BR,
>>> >> >
>>> >> > Alan
>>> >> >
>>> >> > On 4/1/22, Petro Karashchenko  wrote:
>>> >> > > Hi,
>>> >> > >
>>> >> > > I want to resume this thread again because after reexamined code
>>> >> > carefully
>>> >> > > I found that VFS layer has an API
>>> >> > >
>>> >> > > int inode_checkflags(FAR struct inode *inode, int oflags)
>>> >> > > {
>>> >> > >   if (((oflags & O_RDOK) != 0 && !inode->u.i_ops->read) ||
>>> >> > >   ((oflags & O_WROK) != 0 && !inode->u.i_ops->write))
>>> >> > > {
>>> >> > >   return -EACCES;
>>> >> > > }
>>> >> > >   else
>>> >> > > {
>>> >> > >   return OK;
>>> >> > > }
>>> >> > > }
>>> >> > >
>>> >> > > That checks if read and write handlers are available, so all our
>>> >> > discussion
>>> >> > > about R/W mode for IOCTL does not make any sense. We either need
>>> >> > > to
>>> >> > remove
>>> >> > > this check or register VFS nodes with proper permissions and open
>>> >> > > files
>>> >> > > with correct flags. So if the driver does not have neither read
>>> >> > > nor
>>> >> write
>>> >> > > handlers the "" mode should be used and "0" should be used
>>> during
>>> >> > > opening of a file. Or we need to remove "inode_checkflags()".
>>> >> > >
>>> >> > > Best regards,
>>> >> > > Petro
>>> >> > >
>>>

Re: register_driver with 0000 mode

2022-04-01 Thread Jukka Laitinen
Different posix implementations have different values for these flags, so I 
think it is ok not to have the same as what linux has.

Posix (2017) specifies thar exactly one of the following is provided for open: 
O_EXEC, O_RDWR, O_RDONLY, O_SEARCH and O_WRONLY, and other flags are bitwise 
OR'd to that. The spec says nothing about that you can just give "0" afaik, it 
just happens to be that in Linux O_RDONLY happens to be 0.

Maybe just fix the open having O_RDONLY in places where you really open the 
file as read-only, O_WRONLY for write only and O? That should be according to 
the standard at least.

Just my 2 cents

- Jukka



Alan Carvalho de Assis kirjoitti perjantai 1. huhtikuuta 2022:
> More about it here:
> https://stackoverflow.com/questions/61923703/how-to-make-sense-of-o-rdonly-0
> 
> So, I agree with the comment that said "calling it flag is misnomer
> and misleading"
> 
> On Unix/Linux O_RDONLY | O_WRONLY != O_RDWR, on NuttX is it explicitly this 
> way:
> 
> #define O_RDWR  (O_RDOK|O_WROK) /* Open for both read & write access */
> 
> Now, I'm also confuse about the right thing to do:
> 
> 1) Fix include/fcntl.h to follow Unix/Linux
> 2) Keep things we they are and don't accept opening a file if it
> doesn't have RDONLY flag (and change all the drivers, event ioctl only
> drivers, to include reading flag)
> 3) Remove the flag checking.
> 
> Probably we should do 1) because NuttX follows Unix/Linux approach,
> although I agree that Unix/Linux are completely non-sense on this
> subject, oflag should be a flag like it is on NuttX :-)
> 
> BR,
> 
> Alan
> 
> On 4/1/22, Alan Carvalho de Assis  wrote:
> > Hmm, I think oflag equal 0 on Unix(so MacOS)/Linux works because of it:
> >
> > #define O_RDONLY
> >
> > So, in fact on Linux/Mac when we are opening a file with oflag 0 we
> > are opening it for reading only.
> >
> > On NuttX the value 0 is not defined, O_RDONLY is 1:
> >
> > #define O_RDONLY(1 << 0)
> >
> > BR,
> >
> > Alan
> >
> > On 4/1/22, Petro Karashchenko  wrote:
> >> Hello,
> >>
> >> Here I'm talking not about driver registration permission, but more about
> >> the "oflag" parameter to "open()" call.
> >>
> >> I just tried a quick example on MAC
> >>
> >> #include 
> >> #include 
> >>
> >> int main(void)
> >> {
> >>   int fd = open("test.txt", 0);
> >>   if (fd < 0)
> >> printf("A\n");
> >>   else
> >> printf("B\n");
> >>
> >>   return 0;
> >> }
> >>
> >> The "B" is printed if the file exists. If you know the system that will
> >> run
> >> this sample code and will print "A", please let me know.
> >>
> >> Best regards,
> >> Petro
> >>
> >> пт, 1 квіт. 2022 р. о 16:27 Alan Carvalho de Assis 
> >> пише:
> >>
> >>> I think the device file shouldn't be created with permission 000.
> >>>
> >>> Look inside your Linux /dev all device files have RW permission for
> >>> root, some give only R for group and others.
> >>>
> >>> So, probably we need to fix the device register creation, not removing
> >>> the flag check.
> >>>
> >>> BR,
> >>>
> >>> Alan
> >>>
> >>> On 4/1/22, Xiang Xiao  wrote:
> >>> > It's better to check ioctl callback too since ioctl means the driver
> >>> > has
> >>> > the compatibility of read(i)and write(o).
> >>> >
> >>> > On Fri, Apr 1, 2022 at 9:15 PM Petro Karashchenko <
> >>> > petro.karashche...@gmail.com> wrote:
> >>> >
> >>> >> So Alan do you suggest to remove inode_checkflags?
> >>> >>
> >>> >> On Fri, Apr 1, 2022, 4:13 PM Alan Carvalho de Assis
> >>> >> 
> >>> >> wrote:
> >>> >>
> >>> >> > Hi Petro,
> >>> >> >
> >>> >> > I saw your PR #1117 but I think opening a device file with flag 0
> >>> >> > is
> >>> >> > not correct, please see the open man-pages:
> >>> >> >
> >>> >> > alan@dev:/tmp$ man 2 open
> >>> >> >
> >>> >> >The argument flags must include one of the  following
> >>> >> > access
> >>> >> > modes:  O_RDONLY,  O_WRONLY,  or
> >>> >> >O_RDWR.  These request opening the file read-only,
> >>> >> > write-only,
> >>> >> > or read/write, respectively.
> >>> >> >
> >>> >> > Also the opengroup say something similar:
> >>> >> >
> >>> >> > https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html
> >>> >> >
> >>> >> > "Values for oflag are constructed by a bitwise-inclusive OR of
> >>> >> > flags
> >>> >> > from the following list, defined in . Applications shall
> >>> >> > specify exactly one of the first five values (file access modes)
> >>> >> > below
> >>> >> > in the value of oflag:"
> >>> >> >
> >>> >> > The man pages uses "MUST", the OpenGroups uses "SHALL", but
> >>> >> > according
> >>> >> > to RFC2119 they are equivalents:
> >>> >> >
> >>> >> > https://www.ietf.org/rfc/rfc2119.txt
> >>> >> >
> >>> >> > BR,
> >>> >> >
> >>> >> > Alan
> >>> >> >
> >>> >> > On 4/1/22, Petro Karashchenko  wrote:
> >>> >> > > Hi,
> >>> >> > >
> >>> >> > > I want to resume this thread again because after reexamined code
> >>> >> > carefully
> >>> >> > > I found that VFS layer has an API
> >>> >> > >
> >>> >> > > int inode_chec

Re: register_driver with 0000 mode

2022-04-01 Thread Petro Karashchenko
Yes Jukka what you are saying is absolutely correct. The main item under
discussion are the drivers that are pure ioctl (that means do not have
neither read nor write handlers). Should RD or WR flag be passed to open
call in such case of or 0 should be passed.

Best regards,
Petro

On Fri, Apr 1, 2022, 6:54 PM Jukka Laitinen  wrote:

> Different posix implementations have different values for these flags, so
> I think it is ok not to have the same as what linux has.
>
> Posix (2017) specifies thar exactly one of the following is provided for
> open: O_EXEC, O_RDWR, O_RDONLY, O_SEARCH and O_WRONLY, and other flags are
> bitwise OR'd to that. The spec says nothing about that you can just give
> "0" afaik, it just happens to be that in Linux O_RDONLY happens to be 0.
>
> Maybe just fix the open having O_RDONLY in places where you really open
> the file as read-only, O_WRONLY for write only and O? That should be
> according to the standard at least.
>
> Just my 2 cents
>
> - Jukka
>
>
>
> Alan Carvalho de Assis kirjoitti perjantai 1. huhtikuuta 2022:
> > More about it here:
> >
> https://stackoverflow.com/questions/61923703/how-to-make-sense-of-o-rdonly-0
> >
> > So, I agree with the comment that said "calling it flag is misnomer
> > and misleading"
> >
> > On Unix/Linux O_RDONLY | O_WRONLY != O_RDWR, on NuttX is it explicitly
> this way:
> >
> > #define O_RDWR  (O_RDOK|O_WROK) /* Open for both read & write access
> */
> >
> > Now, I'm also confuse about the right thing to do:
> >
> > 1) Fix include/fcntl.h to follow Unix/Linux
> > 2) Keep things we they are and don't accept opening a file if it
> > doesn't have RDONLY flag (and change all the drivers, event ioctl only
> > drivers, to include reading flag)
> > 3) Remove the flag checking.
> >
> > Probably we should do 1) because NuttX follows Unix/Linux approach,
> > although I agree that Unix/Linux are completely non-sense on this
> > subject, oflag should be a flag like it is on NuttX :-)
> >
> > BR,
> >
> > Alan
> >
> > On 4/1/22, Alan Carvalho de Assis  wrote:
> > > Hmm, I think oflag equal 0 on Unix(so MacOS)/Linux works because of it:
> > >
> > > #define O_RDONLY
> > >
> > > So, in fact on Linux/Mac when we are opening a file with oflag 0 we
> > > are opening it for reading only.
> > >
> > > On NuttX the value 0 is not defined, O_RDONLY is 1:
> > >
> > > #define O_RDONLY(1 << 0)
> > >
> > > BR,
> > >
> > > Alan
> > >
> > > On 4/1/22, Petro Karashchenko  wrote:
> > >> Hello,
> > >>
> > >> Here I'm talking not about driver registration permission, but more
> about
> > >> the "oflag" parameter to "open()" call.
> > >>
> > >> I just tried a quick example on MAC
> > >>
> > >> #include 
> > >> #include 
> > >>
> > >> int main(void)
> > >> {
> > >>   int fd = open("test.txt", 0);
> > >>   if (fd < 0)
> > >> printf("A\n");
> > >>   else
> > >> printf("B\n");
> > >>
> > >>   return 0;
> > >> }
> > >>
> > >> The "B" is printed if the file exists. If you know the system that
> will
> > >> run
> > >> this sample code and will print "A", please let me know.
> > >>
> > >> Best regards,
> > >> Petro
> > >>
> > >> пт, 1 квіт. 2022 р. о 16:27 Alan Carvalho de Assis  >
> > >> пише:
> > >>
> > >>> I think the device file shouldn't be created with permission 000.
> > >>>
> > >>> Look inside your Linux /dev all device files have RW permission for
> > >>> root, some give only R for group and others.
> > >>>
> > >>> So, probably we need to fix the device register creation, not
> removing
> > >>> the flag check.
> > >>>
> > >>> BR,
> > >>>
> > >>> Alan
> > >>>
> > >>> On 4/1/22, Xiang Xiao  wrote:
> > >>> > It's better to check ioctl callback too since ioctl means the
> driver
> > >>> > has
> > >>> > the compatibility of read(i)and write(o).
> > >>> >
> > >>> > On Fri, Apr 1, 2022 at 9:15 PM Petro Karashchenko <
> > >>> > petro.karashche...@gmail.com> wrote:
> > >>> >
> > >>> >> So Alan do you suggest to remove inode_checkflags?
> > >>> >>
> > >>> >> On Fri, Apr 1, 2022, 4:13 PM Alan Carvalho de Assis
> > >>> >> 
> > >>> >> wrote:
> > >>> >>
> > >>> >> > Hi Petro,
> > >>> >> >
> > >>> >> > I saw your PR #1117 but I think opening a device file with flag
> 0
> > >>> >> > is
> > >>> >> > not correct, please see the open man-pages:
> > >>> >> >
> > >>> >> > alan@dev:/tmp$ man 2 open
> > >>> >> >
> > >>> >> >The argument flags must include one of the  following
> > >>> >> > access
> > >>> >> > modes:  O_RDONLY,  O_WRONLY,  or
> > >>> >> >O_RDWR.  These request opening the file read-only,
> > >>> >> > write-only,
> > >>> >> > or read/write, respectively.
> > >>> >> >
> > >>> >> > Also the opengroup say something similar:
> > >>> >> >
> > >>> >> >
> https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html
> > >>> >> >
> > >>> >> > "Values for oflag are constructed by a bitwise-inclusive OR of
> > >>> >> > flags
> > >>> >> > from the following list, defined in . Applications
> shall
> > >>> >> > specify exactly one of the first fi

Re: register_driver with 0000 mode

2022-04-01 Thread Jukka Laitinen
In my opinion 0, if you are asking that, but it is strictly not conforming the 
standard.

Posix says that "Applications shall specify exactly one of the first five...", 
so there is no correct standard conforming way. All five would be wrong, imho.

-Jukka

Petro Karashchenko kirjoitti perjantai 1. huhtikuuta 2022:
> Yes Jukka what you are saying is absolutely correct. The main item under
> discussion are the drivers that are pure ioctl (that means do not have
> neither read nor write handlers). Should RD or WR flag be passed to open
> call in such case of or 0 should be passed.
> 
> Best regards,
> Petro
> 
> On Fri, Apr 1, 2022, 6:54 PM Jukka Laitinen  wrote:
> 
> > Different posix implementations have different values for these flags, so
> > I think it is ok not to have the same as what linux has.
> >
> > Posix (2017) specifies thar exactly one of the following is provided for
> > open: O_EXEC, O_RDWR, O_RDONLY, O_SEARCH and O_WRONLY, and other flags are
> > bitwise OR'd to that. The spec says nothing about that you can just give
> > "0" afaik, it just happens to be that in Linux O_RDONLY happens to be 0.
> >
> > Maybe just fix the open having O_RDONLY in places where you really open
> > the file as read-only, O_WRONLY for write only and O? That should be
> > according to the standard at least.
> >
> > Just my 2 cents
> >
> > - Jukka
> >
> >
> >
> > Alan Carvalho de Assis kirjoitti perjantai 1. huhtikuuta 2022:
> > > More about it here:
> > >
> > https://stackoverflow.com/questions/61923703/how-to-make-sense-of-o-rdonly-0
> > >
> > > So, I agree with the comment that said "calling it flag is misnomer
> > > and misleading"
> > >
> > > On Unix/Linux O_RDONLY | O_WRONLY != O_RDWR, on NuttX is it explicitly
> > this way:
> > >
> > > #define O_RDWR  (O_RDOK|O_WROK) /* Open for both read & write access
> > */
> > >
> > > Now, I'm also confuse about the right thing to do:
> > >
> > > 1) Fix include/fcntl.h to follow Unix/Linux
> > > 2) Keep things we they are and don't accept opening a file if it
> > > doesn't have RDONLY flag (and change all the drivers, event ioctl only
> > > drivers, to include reading flag)
> > > 3) Remove the flag checking.
> > >
> > > Probably we should do 1) because NuttX follows Unix/Linux approach,
> > > although I agree that Unix/Linux are completely non-sense on this
> > > subject, oflag should be a flag like it is on NuttX :-)
> > >
> > > BR,
> > >
> > > Alan
> > >
> > > On 4/1/22, Alan Carvalho de Assis  wrote:
> > > > Hmm, I think oflag equal 0 on Unix(so MacOS)/Linux works because of it:
> > > >
> > > > #define O_RDONLY
> > > >
> > > > So, in fact on Linux/Mac when we are opening a file with oflag 0 we
> > > > are opening it for reading only.
> > > >
> > > > On NuttX the value 0 is not defined, O_RDONLY is 1:
> > > >
> > > > #define O_RDONLY(1 << 0)
> > > >
> > > > BR,
> > > >
> > > > Alan
> > > >
> > > > On 4/1/22, Petro Karashchenko  wrote:
> > > >> Hello,
> > > >>
> > > >> Here I'm talking not about driver registration permission, but more
> > about
> > > >> the "oflag" parameter to "open()" call.
> > > >>
> > > >> I just tried a quick example on MAC
> > > >>
> > > >> #include 
> > > >> #include 
> > > >>
> > > >> int main(void)
> > > >> {
> > > >>   int fd = open("test.txt", 0);
> > > >>   if (fd < 0)
> > > >> printf("A\n");
> > > >>   else
> > > >> printf("B\n");
> > > >>
> > > >>   return 0;
> > > >> }
> > > >>
> > > >> The "B" is printed if the file exists. If you know the system that
> > will
> > > >> run
> > > >> this sample code and will print "A", please let me know.
> > > >>
> > > >> Best regards,
> > > >> Petro
> > > >>
> > > >> пт, 1 квіт. 2022 р. о 16:27 Alan Carvalho de Assis  > >
> > > >> пише:
> > > >>
> > > >>> I think the device file shouldn't be created with permission 000.
> > > >>>
> > > >>> Look inside your Linux /dev all device files have RW permission for
> > > >>> root, some give only R for group and others.
> > > >>>
> > > >>> So, probably we need to fix the device register creation, not
> > removing
> > > >>> the flag check.
> > > >>>
> > > >>> BR,
> > > >>>
> > > >>> Alan
> > > >>>
> > > >>> On 4/1/22, Xiang Xiao  wrote:
> > > >>> > It's better to check ioctl callback too since ioctl means the
> > driver
> > > >>> > has
> > > >>> > the compatibility of read(i)and write(o).
> > > >>> >
> > > >>> > On Fri, Apr 1, 2022 at 9:15 PM Petro Karashchenko <
> > > >>> > petro.karashche...@gmail.com> wrote:
> > > >>> >
> > > >>> >> So Alan do you suggest to remove inode_checkflags?
> > > >>> >>
> > > >>> >> On Fri, Apr 1, 2022, 4:13 PM Alan Carvalho de Assis
> > > >>> >> 
> > > >>> >> wrote:
> > > >>> >>
> > > >>> >> > Hi Petro,
> > > >>> >> >
> > > >>> >> > I saw your PR #1117 but I think opening a device file with flag
> > 0
> > > >>> >> > is
> > > >>> >> > not correct, please see the open man-pages:
> > > >>> >> >
> > > >>> >> > alan@dev:/tmp$ man 2 open
> > > >>> >> >
> > > >>> >> >The argument flags must include one of 

Re: register_driver with 0000 mode

2022-04-01 Thread Jukka Laitinen
One option, conforming standard, would be that you just always give O_RDWR 
(same flags as what linux devices have), but then when calling read/write you 
check if the pointer is non-null. If the driver doesn't define read or write, 
those operations are allowed on the device, but act as no-op.

- Jukka

Jukka Laitinen kirjoitti perjantai 1. huhtikuuta 2022:
> In my opinion 0, if you are asking that, but it is strictly not conforming 
> the standard.
> 
> Posix says that "Applications shall specify exactly one of the first 
> five...", so there is no correct standard conforming way. All five would be 
> wrong, imho.
> 
> -Jukka
> 
> Petro Karashchenko kirjoitti perjantai 1. huhtikuuta 2022:
> > Yes Jukka what you are saying is absolutely correct. The main item under
> > discussion are the drivers that are pure ioctl (that means do not have
> > neither read nor write handlers). Should RD or WR flag be passed to open
> > call in such case of or 0 should be passed.
> > 
> > Best regards,
> > Petro
> > 
> > On Fri, Apr 1, 2022, 6:54 PM Jukka Laitinen  wrote:
> > 
> > > Different posix implementations have different values for these flags, so
> > > I think it is ok not to have the same as what linux has.
> > >
> > > Posix (2017) specifies thar exactly one of the following is provided for
> > > open: O_EXEC, O_RDWR, O_RDONLY, O_SEARCH and O_WRONLY, and other flags are
> > > bitwise OR'd to that. The spec says nothing about that you can just give
> > > "0" afaik, it just happens to be that in Linux O_RDONLY happens to be 0.
> > >
> > > Maybe just fix the open having O_RDONLY in places where you really open
> > > the file as read-only, O_WRONLY for write only and O? That should be
> > > according to the standard at least.
> > >
> > > Just my 2 cents
> > >
> > > - Jukka
> > >
> > >
> > >
> > > Alan Carvalho de Assis kirjoitti perjantai 1. huhtikuuta 2022:
> > > > More about it here:
> > > >
> > > https://stackoverflow.com/questions/61923703/how-to-make-sense-of-o-rdonly-0
> > > >
> > > > So, I agree with the comment that said "calling it flag is misnomer
> > > > and misleading"
> > > >
> > > > On Unix/Linux O_RDONLY | O_WRONLY != O_RDWR, on NuttX is it explicitly
> > > this way:
> > > >
> > > > #define O_RDWR  (O_RDOK|O_WROK) /* Open for both read & write access
> > > */
> > > >
> > > > Now, I'm also confuse about the right thing to do:
> > > >
> > > > 1) Fix include/fcntl.h to follow Unix/Linux
> > > > 2) Keep things we they are and don't accept opening a file if it
> > > > doesn't have RDONLY flag (and change all the drivers, event ioctl only
> > > > drivers, to include reading flag)
> > > > 3) Remove the flag checking.
> > > >
> > > > Probably we should do 1) because NuttX follows Unix/Linux approach,
> > > > although I agree that Unix/Linux are completely non-sense on this
> > > > subject, oflag should be a flag like it is on NuttX :-)
> > > >
> > > > BR,
> > > >
> > > > Alan
> > > >
> > > > On 4/1/22, Alan Carvalho de Assis  wrote:
> > > > > Hmm, I think oflag equal 0 on Unix(so MacOS)/Linux works because of 
> > > > > it:
> > > > >
> > > > > #define O_RDONLY
> > > > >
> > > > > So, in fact on Linux/Mac when we are opening a file with oflag 0 we
> > > > > are opening it for reading only.
> > > > >
> > > > > On NuttX the value 0 is not defined, O_RDONLY is 1:
> > > > >
> > > > > #define O_RDONLY(1 << 0)
> > > > >
> > > > > BR,
> > > > >
> > > > > Alan
> > > > >
> > > > > On 4/1/22, Petro Karashchenko  wrote:
> > > > >> Hello,
> > > > >>
> > > > >> Here I'm talking not about driver registration permission, but more
> > > about
> > > > >> the "oflag" parameter to "open()" call.
> > > > >>
> > > > >> I just tried a quick example on MAC
> > > > >>
> > > > >> #include 
> > > > >> #include 
> > > > >>
> > > > >> int main(void)
> > > > >> {
> > > > >>   int fd = open("test.txt", 0);
> > > > >>   if (fd < 0)
> > > > >> printf("A\n");
> > > > >>   else
> > > > >> printf("B\n");
> > > > >>
> > > > >>   return 0;
> > > > >> }
> > > > >>
> > > > >> The "B" is printed if the file exists. If you know the system that
> > > will
> > > > >> run
> > > > >> this sample code and will print "A", please let me know.
> > > > >>
> > > > >> Best regards,
> > > > >> Petro
> > > > >>
> > > > >> пт, 1 квіт. 2022 р. о 16:27 Alan Carvalho de Assis  > > >
> > > > >> пише:
> > > > >>
> > > > >>> I think the device file shouldn't be created with permission 000.
> > > > >>>
> > > > >>> Look inside your Linux /dev all device files have RW permission for
> > > > >>> root, some give only R for group and others.
> > > > >>>
> > > > >>> So, probably we need to fix the device register creation, not
> > > removing
> > > > >>> the flag check.
> > > > >>>
> > > > >>> BR,
> > > > >>>
> > > > >>> Alan
> > > > >>>
> > > > >>> On 4/1/22, Xiang Xiao  wrote:
> > > > >>> > It's better to check ioctl callback too since ioctl means the
> > > driver
> > > > >>> > has
> > > > >>> > the compatibility of read(i)and write(o).
> > > > >>> >

PR 5782 and code reviee process

2022-04-01 Thread Jukka Laitinen


Hi,
 
If RISC-V S-mode and CONFIG_BUILD_KERNEL support is not wanted into NuttX, 
please say it out loud instead of playing unfair review games.
 
The team has better things to do than re-write code letter-by letter via code 
review, week after week. The code is good quality and working, and is done 
according to nuttx standards.
 
We have always the option to keep it all ourselves and branch off. We do want 
to contribute, but if there is no will to receive code, we can change the 
policy and just bring in small bugfixes in the future and let the fundamental 
larger improvements stay in our own repositories.
 
If you want to prettify things to look better to your eye, just make another PR 
*yourself* *after* merging the working, well inspected and approved PR.
 
Thanks for your input for this issue,
Jukka


Re: PR 5782 and code reviee process

2022-04-01 Thread Alan Carvalho de Assis
Hi Jukka,

I think everybody want RISC-V with Kernel mode support on NuttX.

Sometimes we get really angry because some comments seem too picky,
but we need to believe in the "The Wisdom of Crowds" (i.e.:
https://www.youtube.com/watch?v=Qfh-k9P8ZPI )

So, instead getting mad about it, please try to implement the
suggestions. You are an experienced contributor and I'm sure you can
get this PR accepted as well. :-)

BR,

Alan

On 4/1/22, Jukka Laitinen  wrote:
>
>
> Hi,
>
> If RISC-V S-mode and CONFIG_BUILD_KERNEL support is not wanted into NuttX,
> please say it out loud instead of playing unfair review games.
>
> The team has better things to do than re-write code letter-by letter via
> code review, week after week. The code is good quality and working, and is
> done according to nuttx standards.
>
> We have always the option to keep it all ourselves and branch off. We do
> want to contribute, but if there is no will to receive code, we can change
> the policy and just bring in small bugfixes in the future and let the
> fundamental larger improvements stay in our own repositories.
>
> If you want to prettify things to look better to your eye, just make another
> PR *yourself* *after* merging the working, well inspected and approved PR.
>
> Thanks for your input for this issue,
> Jukka
>


Re: register_driver with 0000 mode

2022-04-01 Thread Gregory Nutt



One option, conforming standard, would be that you just always give O_RDWR 
(same flags as what linux devices have), but then when calling read/write you 
check if the pointer is non-null. If the driver doesn't define read or write, 
those operations are allowed on the device, but act as no-op.


If you can't think of anything useful to do with read() or write(),  
thenthis has been historically handled is by including a dummy read 
method in the driver that just returns zero (EOF).  For example, the 
loop driver:


/
 * Name: loop_read
 /

static ssize_t loop_read(FAR struct file *filep, FAR char *buffer,
 size_t len)
{
  return 0; /* Return EOF */
}
*
*


Re: PR 5782 and code reviee process

2022-04-01 Thread Jukka Laitinen
I am not the author there, just feel that the process is not fair for the 
author.

The PR has over 300 comments, and they come in bunches of 20, every day one 
bunch more. Most of them are just of type "please replace xxxyyy with xxx_yyy", 
or "please move these lines 20 lines above". That is, purely subjective "I 
think this looks better if..".

The author (who I know and have very high respect for) has been re-basing this 
PR and all the work on top of it now for weeks, since while delaying this PR, 
there are other breaking stuff being pushed in with fast pace by the company of 
the main reviewer.

Earlier this week, Petro K. did some good work with the author and everyone 
seemed happy, the PR was approved. But after approval it didn't get merged, but 
received another 20 "maybe it looks better this way" from the other reviewer!

- Jukka

Alan Carvalho de Assis kirjoitti perjantai 1. huhtikuuta 2022:
> Hi Jukka,
> 
> I think everybody want RISC-V with Kernel mode support on NuttX.
> 
> Sometimes we get really angry because some comments seem too picky,
> but we need to believe in the "The Wisdom of Crowds" (i.e.:
> https://www.youtube.com/watch?v=Qfh-k9P8ZPI )
> 
> So, instead getting mad about it, please try to implement the
> suggestions. You are an experienced contributor and I'm sure you can
> get this PR accepted as well. :-)
> 
> BR,
> 
> Alan
> 
> On 4/1/22, Jukka Laitinen  wrote:
> >
> >
> > Hi,
> >
> > If RISC-V S-mode and CONFIG_BUILD_KERNEL support is not wanted into NuttX,
> > please say it out loud instead of playing unfair review games.
> >
> > The team has better things to do than re-write code letter-by letter via
> > code review, week after week. The code is good quality and working, and is
> > done according to nuttx standards.
> >
> > We have always the option to keep it all ourselves and branch off. We do
> > want to contribute, but if there is no will to receive code, we can change
> > the policy and just bring in small bugfixes in the future and let the
> > fundamental larger improvements stay in our own repositories.
> >
> > If you want to prettify things to look better to your eye, just make another
> > PR *yourself* *after* merging the working, well inspected and approved PR.
> >
> > Thanks for your input for this issue,
> > Jukka
> >
>

Re: PR 5782 and code reviee process

2022-04-01 Thread Alan Carvalho de Assis
Fixed!

On 4/1/22, Jukka Laitinen  wrote:
> I am not the author there, just feel that the process is not fair for the
> author.
>
> The PR has over 300 comments, and they come in bunches of 20, every day one
> bunch more. Most of them are just of type "please replace xxxyyy with
> xxx_yyy", or "please move these lines 20 lines above". That is, purely
> subjective "I think this looks better if..".
>
> The author (who I know and have very high respect for) has been re-basing
> this PR and all the work on top of it now for weeks, since while delaying
> this PR, there are other breaking stuff being pushed in with fast pace by
> the company of the main reviewer.
>
> Earlier this week, Petro K. did some good work with the author and everyone
> seemed happy, the PR was approved. But after approval it didn't get merged,
> but received another 20 "maybe it looks better this way" from the other
> reviewer!
>
> - Jukka
>
> Alan Carvalho de Assis kirjoitti perjantai 1. huhtikuuta 2022:
>> Hi Jukka,
>>
>> I think everybody want RISC-V with Kernel mode support on NuttX.
>>
>> Sometimes we get really angry because some comments seem too picky,
>> but we need to believe in the "The Wisdom of Crowds" (i.e.:
>> https://www.youtube.com/watch?v=Qfh-k9P8ZPI )
>>
>> So, instead getting mad about it, please try to implement the
>> suggestions. You are an experienced contributor and I'm sure you can
>> get this PR accepted as well. :-)
>>
>> BR,
>>
>> Alan
>>
>> On 4/1/22, Jukka Laitinen  wrote:
>> >
>> >
>> > Hi,
>> >
>> > If RISC-V S-mode and CONFIG_BUILD_KERNEL support is not wanted into
>> > NuttX,
>> > please say it out loud instead of playing unfair review games.
>> >
>> > The team has better things to do than re-write code letter-by letter
>> > via
>> > code review, week after week. The code is good quality and working, and
>> > is
>> > done according to nuttx standards.
>> >
>> > We have always the option to keep it all ourselves and branch off. We
>> > do
>> > want to contribute, but if there is no will to receive code, we can
>> > change
>> > the policy and just bring in small bugfixes in the future and let the
>> > fundamental larger improvements stay in our own repositories.
>> >
>> > If you want to prettify things to look better to your eye, just make
>> > another
>> > PR *yourself* *after* merging the working, well inspected and approved
>> > PR.
>> >
>> > Thanks for your input for this issue,
>> > Jukka
>> >
>>


Re: PR 5782 and code reviee process

2022-04-01 Thread Xiang Xiao
On Sat, Apr 2, 2022 at 3:16 AM Jukka Laitinen  wrote:

> I am not the author there, just feel that the process is not fair for the
> author.
>
> The PR has over 300 comments, and they come in bunches of 20, every day
> one bunch more. Most of them are just of type "please replace xxxyyy with
> xxx_yyy", or "please move these lines 20 lines above". That is, purely
> subjective "I think this looks better if..".
>
>
Yes, some suggestion is to move the code around the different location, but
it isn't a simple style change, here is an example:

01:  /* Setup arg0(exception cause), arg1(context) */
02:
03:  csrr   a0, CSR_CAUSE/* exception cause */
04:  mv a1, sp   /* context = sp */
05:
06:#if CONFIG_ARCH_INTERRUPTSTACK > 15
07:
08:  /* Offset to hartid */
09:
10:  mv s0, a0   /* save cause */
11:  jalx1, riscv_mhartid/* get hartid */
12:
13:  /* Switch to interrupt stack */
14:
15:#if IRQ_NSTACKS > 1
16:  li t0, (CONFIG_ARCH_INTERRUPTSTACK & ~15)
17:  mult0, a0, t0
18:  la a0, g_intstacktop
19:  subsp, a0, t0
20:#else
21:  la sp, g_intstacktop
22:#endif
23:
24:  mv a0, s0   /* restore cause */
25:
26:  /* Call interrupt handler in C */
27:
28:  jalx1, riscv_dispatch_irq

Line 11 should move after line 15 since one cpu doesn't need to get cpu id.
Line 03 could move just before line 27 to avoid save and restore a0 at line
10 and 24.
>From the below update, you can see the logic is more clear:

01:  /* Setup arg1(context) */
02:
03:  mv a1, sp   /* context = sp */
04:
05:#if CONFIG_ARCH_INTERRUPTSTACK > 15
06:
07:  /* Switch to interrupt stack */
08:
09:#if IRQ_NSTACKS > 1
10:  /* Offset to hartid */
11:
12:  jalx1, riscv_mhartid/* get hartid */
13:
14:  li t0, (CONFIG_ARCH_INTERRUPTSTACK & ~15)
15:  mult0, a0, t0
16:  la a0, g_intstacktop
17:  subsp, a0, t0
18:#else
19:  la sp, g_intstacktop
20:#endif
21:
22:  /* Setup arg0(exception cause) */
23:  csrr   a0, CSR_CAUSE/* exception cause */
24:
25:  /* Call interrupt handler in C */
26:
27:  jalx1, riscv_dispatch_irq

After the review, the commit was reduced from "1,710 additions and 118
deletions" to "967 additions and 192 deletions". If my last comment gets
acknowledged, an additional 100~ lines could be eliminated.
The patch was merged by Alan and all my comments are ignored. What's the
side effect? We can't even pass the compiling if kernel mode is enabled,
because this patch forgets to commit riscv_sbi.c(highlighted in my comment).

The author (who I know and have very high respect for) has been re-basing
> this PR and all the work on top of it now for weeks, since while delaying
> this PR,


Yes, pussuw  and I have a good conversation in
PR, and many issues(addrenv, riscv_doirq) get improved quickly.


> there are other breaking stuff being pushed in with fast pace by the
> company of the main reviewer.
>
>
Do you mean the context switch patch? It doesn't break anything, I think.

Earlier this week, Petro K. did some good work with the author and everyone
> seemed happy, the PR was approved. But after approval it didn't get merged,
> but received another 20 "maybe it looks better this way" from the other
> reviewer!
>

My comment found some more issues not found by Petro, I have shown several
before, it isn't a simple style preference but the quality improvement. The
term " "maybe it looks better this way" may confuse you, please read the
suggestion more closely.
Anyway, since many suggestions could improve the performance and reduce the
code size, I will provide a patch in the next couple of days. Review is
welcome.


>
> - Jukka
>
> Alan Carvalho de Assis kirjoitti perjantai 1. huhtikuuta 2022:
> > Hi Jukka,
> >
> > I think everybody want RISC-V with Kernel mode support on NuttX.
> >
> > Sometimes we get really angry because some comments seem too picky,
> > but we need to believe in the "The Wisdom of Crowds" (i.e.:
> > https://www.youtube.com/watch?v=Qfh-k9P8ZPI )
> >
> > So, instead getting mad about it, please try to implement the
> > suggestions. You are an experienced contributor and I'm sure you can
> > get this PR accepted as well. :-)
> >
> > BR,
> >
> > Alan
> >
> > On 4/1/22, Jukka Laitinen  wrote:
> > >
> > >
> > > Hi,
> > >
> > > If RISC-V S-mode and CONFIG_BUILD_KERNEL support is not wanted into
> NuttX,
> > > please say it out loud instead of playing unfair review games.
> > >
> > > The team has better things to do than re-write code letter-by letter
> via
> > > code review, week after week. The code is good quality and working,
> and is
> > > done according to nuttx standards.
> > >
> > > We have always the option to keep it all ourselves and branch off. We
> do
> > > want to contribute, but if there is no will to receive code, we can
> change
> > > the policy and just bring in small bugfixes 

RE: GSoC 2022 ideas - pysimCoder, silicon-heaven, ORTE Data-Distribution Service (DDS), motion control, CAN

2022-04-01 Thread Jiri Vlasak
> 2) ORTE Data-Distribution Service (DDS)

[...]

> But speak with him on continuation of the work or even
> in general has been intentionally and hard way blocked by professor
> Zdenek Hanzalek and doctor Michal Sojka later when Jirka was
> allocated at proprietary NVIDIA work at university for company
> consortium. By the way that work has been paid from European
> public money.

Not true. I've never worked with/for NVIDIA. The speak was not blocked,
I only moved from "networking" work to "trajectory planning for
automated vehicles" and put ORTE on hold due to lack of time.

When someone would work on ORTE, I'm happy to provide code review if
desired.


Re: register_driver with 0000 mode

2022-04-01 Thread Petro Karashchenko
Hello Gregory,

Do you recommend not to allow registration of a driver that does not have
read method and make a requirement for the driver to provide at least dummy
read?

Best regards,
Petro

On Fri, Apr 1, 2022, 9:48 PM Gregory Nutt  wrote:

>
> > One option, conforming standard, would be that you just always give
> O_RDWR (same flags as what linux devices have), but then when calling
> read/write you check if the pointer is non-null. If the driver doesn't
> define read or write, those operations are allowed on the device, but act
> as no-op.
>
> If you can't think of anything useful to do with read() or write(),
> thenthis has been historically handled is by including a dummy read
> method in the driver that just returns zero (EOF).  For example, the
> loop driver:
>
>
> /
>   * Name: loop_read
>
>   
> /
>
> static ssize_t loop_read(FAR struct file *filep, FAR char *buffer,
>   size_t len)
> {
>return 0; /* Return EOF */
> }
> *
> *
>


Re: register_driver with 0000 mode

2022-04-01 Thread Jukka Laitinen
As Greg said, this is a traditional way. I'd maybe allow dummy reads/writes if 
permissions allow, but keep that logic in vfs by checking if the pointer is 
initialized. Then you don't need to provide the dummy implementation in every 
driver. 

Petro Karashchenko kirjoitti lauantai 2. huhtikuuta 2022:
> Hello Gregory,
> 
> Do you recommend not to allow registration of a driver that does not have
> read method and make a requirement for the driver to provide at least dummy
> read?
> 
> Best regards,
> Petro
> 
> On Fri, Apr 1, 2022, 9:48 PM Gregory Nutt  wrote:
> 
> >
> > > One option, conforming standard, would be that you just always give
> > O_RDWR (same flags as what linux devices have), but then when calling
> > read/write you check if the pointer is non-null. If the driver doesn't
> > define read or write, those operations are allowed on the device, but act
> > as no-op.
> >
> > If you can't think of anything useful to do with read() or write(),
> > thenthis has been historically handled is by including a dummy read
> > method in the driver that just returns zero (EOF).  For example, the
> > loop driver:
> >
> >
> > /
> >   * Name: loop_read
> >
> >   
> > /
> >
> > static ssize_t loop_read(FAR struct file *filep, FAR char *buffer,
> >   size_t len)
> > {
> >return 0; /* Return EOF */
> > }
> > *
> > *
> >
>