You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/04/26 09:01:04 UTC
[GitHub] [incubator-nuttx] maht opened a new pull request, #6152: libc/stdio: Add stdio file locking functions
maht opened a new pull request, #6152:
URL: https://github.com/apache/incubator-nuttx/pull/6152
## Summary
Add flockfile(), ftrylockfile() and funlockfile() functions [1]
when the CONFIG_STDIO_DISABLE_BUFFERING is not set.
[1] POSIX.1-2008 / System Interfaces / flockfile
https://pubs.opengroup.org/onlinepubs/9699919799.2008edition/functions/flockfile.html
## Impact
Extending current interfaces (backward compatible).
## Testing
Probably some tests are needed to be added to `incubator-nuttx-apps`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] maht commented on a diff in pull request #6152: libc/stdio: Add stdio file locking functions
Posted by GitBox <gi...@apache.org>.
maht commented on code in PR #6152:
URL: https://github.com/apache/incubator-nuttx/pull/6152#discussion_r861656152
##########
libs/libc/stdio/lib_ftrylockfile.c:
##########
@@ -0,0 +1,100 @@
+/****************************************************************************
+ * libs/libc/stdio/lib_ftrylockfile.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+
+#include <sys/types.h>
+#include <unistd.h>
+#include <errno.h>
+#include <assert.h>
+
+#include <nuttx/semaphore.h>
+#include <nuttx/fs/fs.h>
+
+#include "libc.h"
+
+#ifndef CONFIG_STDIO_DISABLE_BUFFERING
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: _take_semaphore_nonblocking
+ *
+ * Description:
+ * Take a semaphore if its available, returning inmediately if not.
+ *
+ * Input Parameters:
+ * sem - The semaphore.
+ *
+ * Returned Value:
+ * OK - The semaphore is taken successfully.
+ * -EAGAIN - The semaphore is not available.
+ * -EINVAL - Invalid attempt to get the semaphore.
+ *
+ ****************************************************************************/
+
+static int _take_semaphore_nonblocking(sem_t *sem)
+{
+ int ret;
+
+ /* Try to take the semaphore (without waiting) */
+
+ if ((ret = _SEM_TRYWAIT(sem)) < 0)
+ {
+ DEBUGASSERT(_SEM_ERRNO(ret) == EAGAIN ||
+ _SEM_ERRNO(ret) == EINVAL);
Review Comment:
Because these are the values defined in [`nxsem_trywait()`](https://github.com/apache/incubator-nuttx/blob/dcb440a4d9f953361696a19bc41810247f6dde70/sched/semaphore/sem_trywait.c#L67). But maybe this needs a revision, since `_SEM_TRYWAIT` can be also be defined to [`sem_trywait()`](https://github.com/apache/incubator-nuttx/blob/dcb440a4d9f953361696a19bc41810247f6dde70/sched/semaphore/sem_trywait.c#L137) and then the error is returned in `errno`. I will rework this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] maht commented on pull request #6152: libc/stdio: Add stdio file locking functions
Posted by GitBox <gi...@apache.org>.
maht commented on PR #6152:
URL: https://github.com/apache/incubator-nuttx/pull/6152#issuecomment-1287683964
> @maht do you still plan to work on this sooner or later?
sorry @yamt, I just saw the thread. I totally forgot about this PR, thanks for the notification.
> @maht @yamt could you review the update, it's ready for merging now.
thanks @xiaoxiang781216 for the update changes. It's seems I cannot approve my own commit by GitHub "Approve", but I agree with them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] maht commented on a diff in pull request #6152: libc/stdio: Add stdio file locking functions
Posted by GitBox <gi...@apache.org>.
maht commented on code in PR #6152:
URL: https://github.com/apache/incubator-nuttx/pull/6152#discussion_r861766904
##########
libs/libc/stdio/lib_ftrylockfile.c:
##########
@@ -0,0 +1,100 @@
+/****************************************************************************
+ * libs/libc/stdio/lib_ftrylockfile.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+
+#include <sys/types.h>
+#include <unistd.h>
+#include <errno.h>
+#include <assert.h>
+
+#include <nuttx/semaphore.h>
+#include <nuttx/fs/fs.h>
+
+#include "libc.h"
+
+#ifndef CONFIG_STDIO_DISABLE_BUFFERING
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: _take_semaphore_nonblocking
+ *
+ * Description:
+ * Take a semaphore if its available, returning inmediately if not.
+ *
+ * Input Parameters:
+ * sem - The semaphore.
+ *
+ * Returned Value:
+ * OK - The semaphore is taken successfully.
+ * -EAGAIN - The semaphore is not available.
+ * -EINVAL - Invalid attempt to get the semaphore.
+ *
+ ****************************************************************************/
+
+static int _take_semaphore_nonblocking(sem_t *sem)
+{
+ int ret;
+
+ /* Try to take the semaphore (without waiting) */
+
+ if ((ret = _SEM_TRYWAIT(sem)) < 0)
+ {
+ DEBUGASSERT(_SEM_ERRNO(ret) == EAGAIN ||
+ _SEM_ERRNO(ret) == EINVAL);
Review Comment:
Oh, well spotted. I just used the existing code and it seems to be missing already: https://github.com/apache/incubator-nuttx/blob/dcb440a4d9f953361696a19bc41810247f6dde70/libs/libc/stdio/lib_libfilesem.c#L80-L89 . I will add an extra commit at the beginning/end to address it (unless it is preferable to be done in a separate PR).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6152: libc/stdio: Add stdio file locking functions
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6152:
URL: https://github.com/apache/incubator-nuttx/pull/6152#issuecomment-1284045431
If @maht doesn't have time to finish, @yamt may you continue his work?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on pull request #6152: libc/stdio: Add stdio file locking functions
Posted by GitBox <gi...@apache.org>.
yamt commented on PR #6152:
URL: https://github.com/apache/incubator-nuttx/pull/6152#issuecomment-1283776209
@maht do you still plan to work on this sooner or later?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] maht commented on a diff in pull request #6152: libc/stdio: Add stdio file locking functions
Posted by GitBox <gi...@apache.org>.
maht commented on code in PR #6152:
URL: https://github.com/apache/incubator-nuttx/pull/6152#discussion_r859979918
##########
include/nuttx/lib/lib.h:
##########
@@ -114,10 +114,12 @@ void lib_stream_release(FAR struct task_group_s *group);
#ifdef CONFIG_STDIO_DISABLE_BUFFERING
# define lib_sem_initialize(s)
# define lib_take_semaphore(s)
+# define lib_try_take_semaphore(s)
Review Comment:
yes, maybe keeping the name and the indirection level is of no value and probably the code will become more clear to people familiar with the POSIX naming. I will try to rename it in an extra commit of this draft (this way if desired, it can be collapsed easily into one single commit before making it a non-draft PR).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] maht commented on a diff in pull request #6152: libc/stdio: Add stdio file locking functions
Posted by GitBox <gi...@apache.org>.
maht commented on code in PR #6152:
URL: https://github.com/apache/incubator-nuttx/pull/6152#discussion_r859973593
##########
libs/libc/stdio/lib_libfilesem.c:
##########
@@ -95,6 +95,47 @@ void lib_take_semaphore(FAR struct file_struct *stream)
}
}
+/****************************************************************************
+ * lib_try_take_semaphore
+ ****************************************************************************/
+
+int lib_try_take_semaphore(FAR struct file_struct *stream)
Review Comment:
ok, I was not sure about it, but if you feel it's better I will try to do it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6152: libc/stdio: Add stdio file locking functions
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6152:
URL: https://github.com/apache/incubator-nuttx/pull/6152#discussion_r861493334
##########
include/nuttx/lib/lib.h:
##########
@@ -113,12 +113,11 @@ void lib_stream_release(FAR struct task_group_s *group);
#ifdef CONFIG_STDIO_DISABLE_BUFFERING
# define lib_sem_initialize(s)
-# define lib_take_semaphore(s)
-# define lib_give_semaphore(s)
+# define lib_take_semaphore(s, f)
#else
void lib_sem_initialize(FAR struct file_struct *stream);
-void lib_take_semaphore(FAR struct file_struct *stream);
-void lib_give_semaphore(FAR struct file_struct *stream);
+int lib_take_semaphore(FAR struct file_struct *stream,
Review Comment:
```suggestion
int lib_take_semaphore(FAR struct file_struct *stream,
```
##########
libs/libc/stdio/lib_ftrylockfile.c:
##########
@@ -0,0 +1,100 @@
+/****************************************************************************
+ * libs/libc/stdio/lib_ftrylockfile.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+
+#include <sys/types.h>
+#include <unistd.h>
+#include <errno.h>
+#include <assert.h>
+
+#include <nuttx/semaphore.h>
+#include <nuttx/fs/fs.h>
+
+#include "libc.h"
+
+#ifndef CONFIG_STDIO_DISABLE_BUFFERING
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: _take_semaphore_nonblocking
+ *
+ * Description:
+ * Take a semaphore if its available, returning inmediately if not.
+ *
+ * Input Parameters:
+ * sem - The semaphore.
+ *
+ * Returned Value:
+ * OK - The semaphore is taken successfully.
+ * -EAGAIN - The semaphore is not available.
+ * -EINVAL - Invalid attempt to get the semaphore.
+ *
+ ****************************************************************************/
+
+static int _take_semaphore_nonblocking(sem_t *sem)
+{
+ int ret;
+
+ /* Try to take the semaphore (without waiting) */
+
+ if ((ret = _SEM_TRYWAIT(sem)) < 0)
+ {
+ DEBUGASSERT(_SEM_ERRNO(ret) == EAGAIN ||
+ _SEM_ERRNO(ret) == EINVAL);
Review Comment:
why `EINVAL` is considered as normal operation here, but abnormal in `_take_semaphore_blocking`?
##########
include/stdio.h:
##########
@@ -173,6 +173,12 @@ int setvbuf(FAR FILE *stream, FAR char *buffer, int mode, size_t size);
int ungetc(int c, FAR FILE *stream);
+#ifndef CONFIG_STDIO_DISABLE_BUFFERING
+void flockfile(FAR FILE *stream);
+int ftrylockfile(FAR FILE *stream);
+void funlockfile(FAR FILE *stream);
Review Comment:
```suggestion
void flockfile(FAR FILE *stream);
int ftrylockfile(FAR FILE *stream);
void funlockfile(FAR FILE *stream);
```
##########
libs/libc/stdio/lib_funlockfile.c:
##########
@@ -0,0 +1,81 @@
+/****************************************************************************
+ * libs/libc/stdio/lib_funlockfile.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+
+#include <sys/types.h>
+#include <unistd.h>
+#include <errno.h>
+#include <assert.h>
+
+#include <nuttx/semaphore.h>
+#include <nuttx/fs/fs.h>
+
+#include "libc.h"
+
+#ifndef CONFIG_STDIO_DISABLE_BUFFERING
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: funlockfile
+ *
+ * Description:
+ * funlockfile() relinquish the ownership of a stream granted to the
+ * thread.
+ *
+ * The behavior is undefined if called by a thread other than the current
+ * owner.
+ *
+ ****************************************************************************/
+
+void funlockfile(FAR FILE *stream)
+{
+ /* I better be holding at least one reference to the semaphore */
+
+ DEBUGASSERT(stream->fs_holder == getpid());
+
+ /* Do I hold multiple references to the semphore */
+
+ if (stream->fs_counts > 1)
+ {
+ /* Yes, just release one count and return */
+
+ stream->fs_counts--;
+ }
+ else
+ {
+ /* Nope, this is the last reference I have */
+
+ stream->fs_holder = -1;
Review Comment:
```suggestion
stream->fs_holder = INVALID_PROCESS_ID;
```
##########
libs/libc/stdio/lib_libfilesem.c:
##########
@@ -60,7 +60,8 @@ void lib_sem_initialize(FAR struct file_struct *stream)
* lib_take_semaphore
****************************************************************************/
-void lib_take_semaphore(FAR struct file_struct *stream)
+int lib_take_semaphore(FAR struct file_struct *stream,
Review Comment:
Maybe we can add `flag` param instead of `take_func` and pass `O_NONBLOCK` is case if we need to use `_SEM_TRYWAIT` instead of `_SEM_WAIT`?
@yamt what do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6152: libc/stdio: Add stdio file locking functions
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6152:
URL: https://github.com/apache/incubator-nuttx/pull/6152#issuecomment-1287650900
@maht @yamt could you review the update, it's ready for merging now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] maht commented on a diff in pull request #6152: libc/stdio: Add stdio file locking functions
Posted by GitBox <gi...@apache.org>.
maht commented on code in PR #6152:
URL: https://github.com/apache/incubator-nuttx/pull/6152#discussion_r860310154
##########
libs/libc/stdio/lib_ftrylockfile.c:
##########
@@ -0,0 +1,100 @@
+/****************************************************************************
+ * libs/libc/stdio/lib_ftrylockfile.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+
+#include <sys/types.h>
+#include <unistd.h>
+#include <errno.h>
+#include <assert.h>
+
+#include <nuttx/semaphore.h>
+#include <nuttx/fs/fs.h>
+
+#include "libc.h"
+
+#ifndef CONFIG_STDIO_DISABLE_BUFFERING
+
+/****************************************************************************
+ * Public Functions
Review Comment:
I just saw this error. I will correct it later.
```suggestion
* Private Functions
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] maht commented on a diff in pull request #6152: libc/stdio: Add stdio file locking functions
Posted by GitBox <gi...@apache.org>.
maht commented on code in PR #6152:
URL: https://github.com/apache/incubator-nuttx/pull/6152#discussion_r861651975
##########
include/nuttx/lib/lib.h:
##########
@@ -113,12 +113,11 @@ void lib_stream_release(FAR struct task_group_s *group);
#ifdef CONFIG_STDIO_DISABLE_BUFFERING
# define lib_sem_initialize(s)
-# define lib_take_semaphore(s)
-# define lib_give_semaphore(s)
+# define lib_take_semaphore(s, f)
#else
void lib_sem_initialize(FAR struct file_struct *stream);
-void lib_take_semaphore(FAR struct file_struct *stream);
-void lib_give_semaphore(FAR struct file_struct *stream);
+int lib_take_semaphore(FAR struct file_struct *stream,
Review Comment:
Thanks, I will add it in a fixup commit.
##########
include/stdio.h:
##########
@@ -173,6 +173,12 @@ int setvbuf(FAR FILE *stream, FAR char *buffer, int mode, size_t size);
int ungetc(int c, FAR FILE *stream);
+#ifndef CONFIG_STDIO_DISABLE_BUFFERING
+void flockfile(FAR FILE *stream);
+int ftrylockfile(FAR FILE *stream);
+void funlockfile(FAR FILE *stream);
Review Comment:
Thanks, I will add it in a fixup commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] maht commented on a diff in pull request #6152: libc/stdio: Add stdio file locking functions
Posted by GitBox <gi...@apache.org>.
maht commented on code in PR #6152:
URL: https://github.com/apache/incubator-nuttx/pull/6152#discussion_r861657894
##########
libs/libc/stdio/lib_libfilesem.c:
##########
@@ -60,7 +60,8 @@ void lib_sem_initialize(FAR struct file_struct *stream)
* lib_take_semaphore
****************************************************************************/
-void lib_take_semaphore(FAR struct file_struct *stream)
+int lib_take_semaphore(FAR struct file_struct *stream,
Review Comment:
I was doubting about. Probably for this case is better to pass a flag as you commented and maybe @yamt agrees too. I will change it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6152: libc/stdio: Add stdio file locking functions
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6152:
URL: https://github.com/apache/incubator-nuttx/pull/6152#discussion_r861669650
##########
libs/libc/stdio/lib_ftrylockfile.c:
##########
@@ -0,0 +1,100 @@
+/****************************************************************************
+ * libs/libc/stdio/lib_ftrylockfile.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+
+#include <sys/types.h>
+#include <unistd.h>
+#include <errno.h>
+#include <assert.h>
+
+#include <nuttx/semaphore.h>
+#include <nuttx/fs/fs.h>
+
+#include "libc.h"
+
+#ifndef CONFIG_STDIO_DISABLE_BUFFERING
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: _take_semaphore_nonblocking
+ *
+ * Description:
+ * Take a semaphore if its available, returning inmediately if not.
+ *
+ * Input Parameters:
+ * sem - The semaphore.
+ *
+ * Returned Value:
+ * OK - The semaphore is taken successfully.
+ * -EAGAIN - The semaphore is not available.
+ * -EINVAL - Invalid attempt to get the semaphore.
+ *
+ ****************************************************************************/
+
+static int _take_semaphore_nonblocking(sem_t *sem)
+{
+ int ret;
+
+ /* Try to take the semaphore (without waiting) */
+
+ if ((ret = _SEM_TRYWAIT(sem)) < 0)
+ {
+ DEBUGASSERT(_SEM_ERRNO(ret) == EAGAIN ||
+ _SEM_ERRNO(ret) == EINVAL);
Review Comment:
I mean that `nxsem_wait()` also can return `EINVAL`, but it is not checked `_take_semaphore_blocking `
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a diff in pull request #6152: libc/stdio: Add stdio file locking functions
Posted by GitBox <gi...@apache.org>.
yamt commented on code in PR #6152:
URL: https://github.com/apache/incubator-nuttx/pull/6152#discussion_r859210695
##########
libs/libc/stdio/lib_libfilesem.c:
##########
@@ -95,6 +95,47 @@ void lib_take_semaphore(FAR struct file_struct *stream)
}
}
+/****************************************************************************
+ * lib_try_take_semaphore
+ ****************************************************************************/
+
+int lib_try_take_semaphore(FAR struct file_struct *stream)
Review Comment:
i feel it's better to share code with lib_take_semaphore
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6152: libc/stdio: Add stdio file locking functions
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6152:
URL: https://github.com/apache/incubator-nuttx/pull/6152#discussion_r859894887
##########
include/nuttx/lib/lib.h:
##########
@@ -114,10 +114,12 @@ void lib_stream_release(FAR struct task_group_s *group);
#ifdef CONFIG_STDIO_DISABLE_BUFFERING
# define lib_sem_initialize(s)
# define lib_take_semaphore(s)
+# define lib_try_take_semaphore(s)
Review Comment:
how about rename lib_xxx_semaphore to fxxxlockfile directly?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] maht commented on a diff in pull request #6152: libc/stdio: Add stdio file locking functions
Posted by GitBox <gi...@apache.org>.
maht commented on code in PR #6152:
URL: https://github.com/apache/incubator-nuttx/pull/6152#discussion_r861656908
##########
libs/libc/stdio/lib_funlockfile.c:
##########
@@ -0,0 +1,81 @@
+/****************************************************************************
+ * libs/libc/stdio/lib_funlockfile.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+
+#include <sys/types.h>
+#include <unistd.h>
+#include <errno.h>
+#include <assert.h>
+
+#include <nuttx/semaphore.h>
+#include <nuttx/fs/fs.h>
+
+#include "libc.h"
+
+#ifndef CONFIG_STDIO_DISABLE_BUFFERING
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: funlockfile
+ *
+ * Description:
+ * funlockfile() relinquish the ownership of a stream granted to the
+ * thread.
+ *
+ * The behavior is undefined if called by a thread other than the current
+ * owner.
+ *
+ ****************************************************************************/
+
+void funlockfile(FAR FILE *stream)
+{
+ /* I better be holding at least one reference to the semaphore */
+
+ DEBUGASSERT(stream->fs_holder == getpid());
+
+ /* Do I hold multiple references to the semphore */
+
+ if (stream->fs_counts > 1)
+ {
+ /* Yes, just release one count and return */
+
+ stream->fs_counts--;
+ }
+ else
+ {
+ /* Nope, this is the last reference I have */
+
+ stream->fs_holder = -1;
Review Comment:
Thanks, I will add it at the beginning/end, since this would be a change from original code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #6152: libc/stdio: Add stdio file locking functions
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6152:
URL: https://github.com/apache/incubator-nuttx/pull/6152
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org