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/03/22 18:07:44 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request #5826: tools/jlink: Change the type of pid from uint16_t to uint32_t
xiaoxiang781216 opened a new pull request #5826:
URL: https://github.com/apache/incubator-nuttx/pull/5826
## Summary
Report here: https://github.com/apache/incubator-nuttx/pull/5807
follow up the bellow chnage:
```
commit 0f2f48f8bae0e13f7927ed70c3711d03fe6d5f27
Author: Xiang Xiao <xi...@xiaomi.com>
Date: Sun Mar 20 18:12:26 2022 +0800
sys/type.h: Change pid_t from int16_t to int
to fix the following warning:
include/unistd.h:302:9: error: incompatible redeclaration of library function 'vfork' [-Werror,-Wincompatible-library-redeclaration]
pid_t vfork(void);
^
include/unistd.h:302:9: note: 'vfork' is a builtin with type 'int (void)'
and change 32768 to INT_MAX to match the type change
```
## Impact
jlink
## Testing
--
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 merged pull request #5826: tools/jlink: Change the type of pid from uint16_t to uint32_t
Posted by GitBox <gi...@apache.org>.
pkarashchenko merged pull request #5826:
URL: https://github.com/apache/incubator-nuttx/pull/5826
--
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 #5826: tools/jlink: Change the type of pid from uint16_t to uint32_t
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5826:
URL: https://github.com/apache/incubator-nuttx/pull/5826#issuecomment-1076112586
I would like to postpone the change until it really happen to avoid the unnecessary complex in the current implementation.
--
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 #5826: tools/jlink: Change the type of pid from uint16_t to uint32_t
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5826:
URL: https://github.com/apache/incubator-nuttx/pull/5826#issuecomment-1075848175
> Why not use pid_t directly instead of uint32_t?
>
> e.g.,
>
> ```
> static int get_pid(struct plugin_priv_s *priv, uint32_t idx, pid_t *pid)
> ```
jlink_nuttx.c is built on host side, pid_t typedef on macOS/Linux may different from NuttX.
--
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 edited a comment on pull request #5826: tools/jlink: Change the type of pid from uint16_t to uint32_t
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #5826:
URL: https://github.com/apache/incubator-nuttx/pull/5826#issuecomment-1075852912
> Since `int` is non fixed size type this is probably try for most of the systems, but not all.
Yes from theory, but all arch supported by both JLINK and NuttX is either 32bit or 64bit:
https://www.segger.com/products/debug-probes/j-link/technology/cpus-and-devices/overview-of-supported-cpus-and-devices/
so int will always be 32bit from practice.
--
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 pull request #5826: tools/jlink: Change the type of pid from uint16_t to uint32_t
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5826:
URL: https://github.com/apache/incubator-nuttx/pull/5826#issuecomment-1076095260
Maybe we can introduce a flag that may be passed to host build system and select the size of `pid_t` on a target system? By default it will be 32-bit. Something like
```
#ifdef NUTTX_INT_16BIT
typedef uint16_t nuttx_pid_t;
#else
typedef uint32_t nuttx_pid_t;
#endif
...
if (sizeof(nuttx_pid_t) == sizeof(uint16_t))
{
ret = READU16(priv->pidhash[idx] + priv->tcbinfo->pid_off, pid);
}
else
{
ret = READU32(priv->pidhash[idx] + priv->tcbinfo->pid_off, pid);
}
```
What do you think.
In general I agree that now my suggestion targets only `EFM8` architecture that is supported by J-Link, but not supported by NuttX. So we can leave it for future.
--
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 #5826: tools/jlink: Change the type of pid from uint16_t to uint32_t
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5826:
URL: https://github.com/apache/incubator-nuttx/pull/5826#issuecomment-1075852912
> Since `int` is non fixed size type this is probably try for most of the systems, but not all.
Yes from theory, but all arch supported by both JLINK and NuttX is either 32bit or 64bit, so int will always be 32bit from practice.
--
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