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 2020/04/06 04:42:04 UTC
[GitHub] [incubator-nuttx] YuuichiNakamura opened a new issue #740: problem
with nxsig_timeout (sig_timedwait.c)
YuuichiNakamura opened a new issue #740: problem with nxsig_timeout (sig_timedwait.c)
URL: https://github.com/apache/incubator-nuttx/issues/740
On 3/19/2020 10:38 AM, Sebastian Perta wrote:
> Initially the was reported to me as a problem in the compiler however when I investigated I found there no problem
> In the compiler, the problem was in nuttx/sched/signal/sig_timedwait.c at line 359-360:
>
> wd_start(rtcb->waitdog, waitticks,
> (wdentry_t)nxsig_timeout, 1, wdparm.pvarg);
>
> Looking at the declarations of nxsig_timeout and wdentry_t:
>
> static void nxsig_timeout(int argc, wdparm_t itcb)
> typedef CODE void (*wdentry_t)(int argc, uint32_t arg1, ...);
>
> We can see those declarations are incompatible, if we remove the cast the most compilers (GCC for example) will return
> an warning (other compilers will go as far as to return an error).
> ... warning: passing argument ... from incompatible pointer type [-Wincompatible-pointer-types]
> ... note: expected 'wdentry_t' {aka 'void (*)(int, int, ...)'} but argument is of type 'void (*)(int, int)'
>
> One might think this warning can be safely ignored, especially since it works in many cases, however this is not
> generally true it can fail on some targets depending on ABI specification. This is made clear in the
> ISO/IEC standard chapter 7.6 more exactly the following statement:
> "The rightmost parameter plays a special role in the access mechanism, and will be designated parmN in this description."
>
> What this means is that "itcb" from nxsig_timeout can be at a different location from where "arg1" from wdentry_t is expected it to be.
>
> The target for which I've see this problem is Renesas RX. In case of RX itcb is placed in register R2 while arg1 is placed on the stack.
>
> The majority of functions in the code used in this way (casted to wdentry_t) are variadic as well so this is not a problem in general,
> just in a few cases (nxsig_timeout is the only one I found so far there might be a few others)
> In order to make the code ANSI/ISO compliant nxsig_timeout needs to be made variadic as well:
> static void nxsig_timeout(int argc, wdparm_t itcb, ...)
>
> I hope you agree with my finding and fix it as this will make the code more robust (incompatible pointer assignment is unsafe)
> and more portable (other targets besides Renesas RX in which I have a vested interest).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo closed issue #740: problem with
nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
patacongo closed issue #740: problem with nxsig_timeout (sig_timedwait.c)
URL: https://github.com/apache/incubator-nuttx/issues/740
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] YuuichiNakamura commented on issue #740: problem
with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on issue #740: problem with nxsig_timeout (sig_timedwait.c)
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-609570919
The minimum essential changes for the portability are included in the PR, but
if possible, I think that the API spec of wd_start() should be changed.
Because the current spec is likely to misuse which breaks the portability.
My proposal is, to change the API to:
` int wd_start(WDOG_ID wdog, int32_t delay, wdentry_t wdentry, size_t size, wdparm_t arg);`
And change the typedef of wdentry_t to:
`typedef CODE void (*wdentry_t)(wdparm_t arg);`
Instead of passing the variable arguments, pass the pointer to struct and its size
to wd_start() for multiple arguments.
For the almost all callers which passes only one argument, if size == 0,
arg is not treated as the pointer, but just a single value and it is given to
the callback as it is.
How about the proposal?
I think that it is difficult to change such API spec, but if acceptable,
I'll try to make the new PR for the API change.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo commented on issue #740: problem with
nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #740: problem with nxsig_timeout (sig_timedwait.c)
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-609915795
> I'll try to make the new PR for the API change.
Please don't. I will close it without merging. It is inappropriate and undesirable.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] yamt commented on issue #740: problem with
nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
yamt commented on issue #740: problem with nxsig_timeout (sig_timedwait.c)
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-609590448
@YuuichiNakamura do you mean to make wd_start do memcpy with the given size to save the structure?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo edited a comment on issue #740: problem
with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #740: problem with nxsig_timeout (sig_timedwait.c)
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-609818714
I prefer the existing interface. Not only is the proposed interface very ugly but does not seem to have any purpose other than added complexity. Let's not change this; we are better off without that change.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] yamt commented on issue #740: problem with
nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
yamt commented on issue #740: problem with nxsig_timeout (sig_timedwait.c)
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-609682248
the proposed api looks better than the current one to me.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] YuuichiNakamura commented on issue #740: problem
with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on issue #740: problem with nxsig_timeout (sig_timedwait.c)
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-609562257
I have found a same problem which Sebastian pointed out in another processor.
Not only Renesus RX, for example, AVR has similar ABI spec and it cannot work
with the current code.
So, I have investigated the all callers of wd_start() in the latest NuttX kernel.
There are 206 wd_start() callers and 47 callers of them uses the type casting
from fixed argument function pointer to the variadic function pointer (wdentry_t).
Many of them have no need to fix because they are for the specific architecture
or specific device, but 8 callers in sched/ directory must be
fixed because they are common to all architectures.
7 callers pass the function pointer with only one argument.
They should be fixed just adding periods like:
> static void nxsig_timeout(int argc, wdparm_t itcb, ...)
The only one exception is pthread_condtimedwait() which handles two arguments.
(This is only one use case in all NuttX kernel source which handle
multiple arguments for wd_start() callback.)
To make it portable, va_arg() must be used in pthread_condtimedout().
I will issue the PR for 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] YuuichiNakamura commented on issue #740: problem
with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on issue #740: problem with nxsig_timeout (sig_timedwait.c)
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-610197182
@patacongo I understood. It's ok for me if the PR #741 is accepted.
Thanks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo commented on issue #740: problem with
nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #740: problem with nxsig_timeout (sig_timedwait.c)
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-609819858
> Not only Renesus RX, for example, AVR has similar ABI spec and it cannot work
> with the current code.
That is odd since the AVR architecture has been supported by NuttX for over ten years and has been thoroughly excersizes with no known, oustanding problems. I believe the existing interface works fine with AVR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] YuuichiNakamura commented on issue #740: problem
with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
YuuichiNakamura commented on issue #740: problem with nxsig_timeout (sig_timedwait.c)
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-609632938
Yes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] yamt commented on issue #740: problem with
nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
yamt commented on issue #740: problem with nxsig_timeout (sig_timedwait.c)
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-610209177
@patacongo the current api has at least two issues, besides the one pointed out here.
1. wd_start is called with variable args with various types. like uint32_t. on the other hand, wd_start assumes that they are all wdparm_t.
2. wd_expiration calls wdog->func with variable args, all wdparm_t.
many of actual callback functions assume different types like uint32_t.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo edited a comment on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672937870
> Is it reasonable?
Absolutely NOT! wdparm_t was added to work around the kind of errors that you are tying to re-introduce. What you are proposing will work on most 32- and 64- bit machines but almost no where else and, hence, cannot be permitted. I will close an PR that attempts to pass watchdog paramters using FAR void * when the correct solution is already in place. What you are proposing is just wrong!
If a memory storage location is allocated as a FAR void * it will be 16, 24, 32, or 64 bits wide and in many cases cannot hold a uint32_t. If it is allocated as a wdparm_t it will be 32 or 64 bits in width and is guaranteed to hold a uint32_t or a pointer in ALL cases. You proposal will not work and will break code.
This problem has already been resolved and the correct type is available, please don't be stubborn and harm the system.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672215889
The old saying applies here: "If it is not broken, then don't fix it."
Nothing is broken, no change is necessary. Achieving code stability is really more important than making a change to wd_start().
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672202008
> What controls the "life" of structure pointed to by "arg". It cannot be in a stack and it can become stale if it is in allocated memory. In general, it cannot be a static variable either. If allocated, what frees the structure.
This is not a problem, however, if you copy the data out of the structure and into the wdog_s structure immediately in the wd_start call as is done now. The original structure itself must be assumed to be volatile.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672937870
> Is it reasonable?
Absolutely NOT! wdparm_t was added to work around the kind of errors that you are tying to re-introduce. What you are proposing will work on most 32- and 64- bit machines but no where else and, hence, cannot be permitted. I will close an PR that attempts to pass watchdog paramters using FAR void * when the correct solution is already in place. What you are proposing is just wrong!
If a memory storage location is allocated as a FAR void * it will be 16, 20, 24, 32, or 64 bits wide and in many cases cannot hold a uint32_t. It it is allocated as a wdparm_t it will be 32 or 64 bits in width and is guaranteed to hold a uint32_t or a pointer. You proposal will not work and will break 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672929712
> but can we assume that `sizeof(FAR void *)` is always equals to `sizeof(uintptr_t)` and `>= sizeof(uint32_t)`?
No. that is a false assumption. sizeof(uintptr) can be less than sizeof(uint32_t).. On most 8-bit machines, sizeof(uintptr_t) is or should be 2 bytes. On ez80 it is either 2 or 3 bytes depending on mode. For ez80, you can see:
In include/stdint.h:
/* Integer types capable of holding object pointers
* As a general rule, the size of size_t should be the same as the size of
* uintptr_t: 32-bits on a machine with 32-bit addressing but 64-bits on a
* machine with 64-bit addressing.
*/
typedef _ssize_t intptr_t;
typedef _size_t uintptr_t;
And in arch/z80/include/ez80/types.h:
/* These are the sizes of the standard integer types. NOTE that these type
* names have a leading underscore character. This file will be included
* (indirectly) by include/stdint.h and typedef'ed to the final name without
* the underscore character. This roundabout way of doings things allows
* the stdint.h to be removed from the include/ directory in the event that
* the user prefers to use the definitions provided by their toolchain header
* files
*
* These are the sizes of the types supported by the ZiLOG compiler:
*
* int - 24-bits
* short - 16-bits
* long - 32-bits
* char - 8-bits
* float - 32-bits
*/
...
/* A pointer is 2 or 3 bytes, depending upon if the ez80 is in z80
* compatibility mode or not
*
* Z80 mode - 16 bits
* ADL mode - 24 bits
*/
...
#elif defined(CONFIG_EZ80_Z80MODE)
typedef signed short _ssize_t;
typedef unsigned short _size_t;
#else
typedef signed int _ssize_t;
typedef unsigned int _size_t;
#endif
For AVR, the size of a pointer is different on the I-space bus and on the D-space bus. As I recall, sizeof(FAR void *) is usualy 16-bits but might be as much as 24-bits).
In fact sizeof(uinptr_t) < sizeof(uint32_t) is very common among the support architecures. You must not use FAR void * as a container for arbitrary values. You will break MANY architectures and we cannot permit you to do that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #740: problem with
nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #740: problem with nxsig_timeout (sig_timedwait.c)
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-609818714
I prefer the existing interface.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] yamt commented on issue #740: problem with
nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
yamt commented on issue #740: problem with nxsig_timeout (sig_timedwait.c)
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-610210256
i agree the proposed api is a bit ugly. but it's definitely better than the current api.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo commented on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672195129
> The minimum essential changes for the portability are included in the PR, but
> if possible, I think that the API spec of wd_start() should be changed.
> Because the current spec is likely to misuse which breaks the portability.
>
> My proposal is, to change the API to:
> ` int wd_start(WDOG_ID wdog, int32_t delay, wdentry_t wdentry, size_t size, wdparm_t arg);`
>
> And change the typedef of wdentry_t to:
> `typedef CODE void (*wdentry_t)(wdparm_t arg);`
>
> Instead of passing the variable arguments, pass the pointer to struct and its size
> to wd_start() for multiple arguments.
What controls the "life" of structure pointed to by "arg". It cannot be in a stack and it can become stale if it is in allocated memory. In general, it cannot be a static variable either. If allocated, what frees the structure.
Because of these complexities, I think this is a bad idea. Let's not do this. I vote -1. Why not just:
int wd_start(WDOG_ID wdog, int32_t delay, wdentry_t wdentry, size_t size,wdparm_t arg1, wdparm_t arg2);`
typedef CODE void (*wdentry_t)(wdparm_t arg1, wdparm_t arg2);`
Since those are passed directly be value rather then through a container structure, the interface is simplified and there is no concern for management of the life of an allocated structure.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672867706
> > But don't use type void *. There are problems using type void * with certain architctures (AVR I think) that was fixed by created wdparm_t. Please continue to use wdparm_t.
>
> The argument was originally a uint32_t. I was changed to a union with commit [b8f3bd8](https://github.com/apache/incubator-nuttx/commit/b8f3bd857be74ff78e01d961afbaedfbf17814ed) because there are many architectures where sizeof(void *) < sizeof(uint32_t), MCS51, ez80, etc. So void * is insufficient for passing a uint32_t.
>
Yes, `sizeof(void *)` may small than `sizeof(uint32_t)`, but I am wondering that is it possible that `sizeof(FAR void*) < sizeof(uint32_t)`.
> This was changed again to the current wdparm_t form with commit [3adcae8](https://github.com/apache/incubator-nuttx/commit/3adcae8ffbf247f1038bbb3b8281ea811799031e) because there were issues in passing a union with certain compilers (SDCC at the time).
>
> So, please don't consider using void * instead of wdparm_t. You will definitely break things!
And if `FAR void *` will break things, there are many basic components will break too, e.g.:
```
/* This struct defines the form of an interrupt service routine */
typedef CODE int (*xcpt_t)(int irq, FAR void *context, FAR void *arg);
/* Defines the work callback */
typedef CODE void (*worker_t)(FAR void *arg);
#ifndef __PTHREAD_ADDR_T_DEFINED
typedef FAR void *pthread_addr_t;
#define __PTHREAD_ADDR_T_DEFINED 1
#endif
typedef CODE pthread_addr_t (*pthread_startroutine_t)(pthread_addr_t);
typedef pthread_startroutine_t pthread_func_t;
/* Non-standard convenience definition of signal handling function types.
* These should be used only internally within the NuttX signal logic.
*/
typedef CODE void (*_sa_handler_t)(int signo);
typedef CODE void (*_sa_sigaction_t)(int signo, FAR siginfo_t *siginfo,
FAR void *context);
int on_exit(CODE void (*func)(int, FAR void *), FAR void *arg);
/* thrd_start_t: function pointer type passed to thrd_create */
typedef CODE int (*thrd_start_t)(FAR void *arg);
/* tss_dtor_t: function pointer type used for TSS destructor */
typedef CODE void (*tss_dtor_t)(FAR void *);
```
I don't think without irq/signal/work especially irq, the system can work as expect.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672212866
The inefficiency in the proposal should be considered as well. You once told me that all calls to wd_start() but one provided only a single parameter. it sems wasteful to set up a structure containing that one parameter and passing it to to wd_start().
What is the size argument? There can be no more than CONFIG_WDOG_MAXPARMS arguments and each must be size wdparm_t. A size_t is used to hold the largest possible memory object (in bytes). A count of arguments makes more since and should be just an int.
So If I wanted to pass one argument I would have to do:
wdparm_t wdarg = (wdparm_t)arg;
ret = wd_start(&wdog, delay, entry, sizeof(wdarg), &wdarg);
arg would have to copied to a wdparm_t variable because sizeof(arg) may not be compatible with sizeof(wdparm_t) so you cannnot just pass a pointer to arg.
Then wd_start() would have to divide size by sizeof(wdparm_t) to get the number of arguments?
Versus my suggestion:
ret = wd_start(&wdog, delay, entry, arg, 0);
Since we are passing by value, not reference, arg will be automatically converted to type wdparm_t.
Which looks less "ugly" to you. (We should not be using ugliness as a value for making engineering decisions unless all other things are equal. I think we say ugly because we can't verbal the reasons for preferring one thing over an another.)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672911537
> > And if `FAR void *` will break things, there are many basic components will break too
>
> No, in general no. FAR void * is only a fatal decision if you intend to use it as a "container" to hold variables of other sizes. sizeof(uint32_t) is always 32 bits, but sizeof(void *) is variable on different MCUs from 16 to 64 bits.
Yes, `sizeof(void *)` may become 16bits due to the small memory model on some arch, but can we assume that `sizeof(FAR void *)` is always equals to `sizeof(uintptr_t)` and `>= sizeof(uint32_t)`?
Since for wdog_s case, most caller pass 'FAR void *', and all other pass pid_t(int16_t)/uint8_t, so it's safe.
I decide to select `FAR void *' because all other similar callbacks use `FAR void *`. Is it reasonable?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672257134
> But don't use type void *. There are problems using type void * with certain architctures (AVR I think) that was fixed by created wdparm_t. Please continue to use wdparm_t.
The argument was originally a uint32_t. I was changed to a union with commit b8f3bd857be74ff78e01d961afbaedfbf17814ed because there are many architectures where sizeof(void *) < sizeof(uint32_t), MCS51, ez80, etc. So void * is insufficient for passing a uint32_t.
This was changed again to the current wdparm_t form with commit 3adcae8ffbf247f1038bbb3b8281ea811799031e because there were issues in passing a union with certain compilers (SDCC at the time).
So, please don't consider using void * instead of wdparm_t. You will definitely break things!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672937870
> Is it reasonable?
Absolutely NOT! wdparm_t was added to work around the kind of errors that you are tying to introduce. What you are proposing will work on most 32- and 64- bit machines but no where else and, hence, cannot be permitted. I will close an PR that attempts to pass watchdog paramters using FAR void * when the correct solution is already in place. What you are proposing is just wrong!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672929712
> but can we assume that `sizeof(FAR void *)` is always equals to `sizeof(uintptr_t)` and `>= sizeof(uint32_t)`?
No. that is a false assumption. sizeof(uintptr) can be less than sizeof(uint32_t).. On most 8-bit machines, sizeof(uintptr_t) is or should be 2 bytes. On ez80 it is either 2 or 3 bytes depending on mode. For ez80, you can see:
In include/stdint.h:
/* Integer types capable of holding object pointers
* As a general rule, the size of size_t should be the same as the size of
* uintptr_t: 32-bits on a machine with 32-bit addressing but 64-bits on a
* machine with 64-bit addressing.
*/
typedef _ssize_t intptr_t;
typedef _size_t uintptr_t;
And in arch/z80/include/ez80/types.h:
/* These are the sizes of the standard integer types. NOTE that these type
* names have a leading underscore character. This file will be included
* (indirectly) by include/stdint.h and typedef'ed to the final name without
* the underscore character. This roundabout way of doings things allows
* the stdint.h to be removed from the include/ directory in the event that
* the user prefers to use the definitions provided by their toolchain header
* files
*
* These are the sizes of the types supported by the ZiLOG compiler:
*
* int - 24-bits
* short - 16-bits
* long - 32-bits
* char - 8-bits
* float - 32-bits
*/
...
/* A pointer is 2 or 3 bytes, depending upon if the ez80 is in z80
* compatibility mode or not
*
* Z80 mode - 16 bits
* ADL mode - 24 bits
*/
...
#elif defined(CONFIG_EZ80_Z80MODE)
typedef signed short _ssize_t;
typedef unsigned short _size_t;
#else
typedef signed int _ssize_t;
typedef unsigned int _size_t;
#endif
And in limits.h:
/* A pointer is 2 or 3 bytes, depending upon if the ez80 is in z80
* compatibility mode or not
*
* Z80 mode - 16 bits
* ADL mode - 24 bits
*/
#define PTR_MIN (-PTR_MAX - 1)
#ifdef CONFIG_EZ80_Z80MODE
#define PTR_MAX 32767
#define UPTR_MAX 65535U
#else
#define PTR_MAX 8388607
#define UPTR_MAX 16777215U
#endif
For AVR, the size of a pointer is different on the I-space bus and on the D-space bus. As I recall, sizeof(FAR void *) is usualy 16-bits but might be as much as 24-bits).
In fact sizeof(uinptr_t) < sizeof(uint32_t) is very common among the support architecures. You must not use FAR void * as a container for arbitrary values. You will break MANY architectures and we cannot permit you to do that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672867706
> > But don't use type void *. There are problems using type void * with certain architctures (AVR I think) that was fixed by created wdparm_t. Please continue to use wdparm_t.
>
> The argument was originally a uint32_t. I was changed to a union with commit [b8f3bd8](https://github.com/apache/incubator-nuttx/commit/b8f3bd857be74ff78e01d961afbaedfbf17814ed) because there are many architectures where sizeof(void *) < sizeof(uint32_t), MCS51, ez80, etc. So void * is insufficient for passing a uint32_t.
>
Yes, sizeof(void *) may small than sizeof(uint32_t), but I am wondering that is it possible that sizeof(FAR void*) < sizeof(uint32_t).
> This was changed again to the current wdparm_t form with commit [3adcae8](https://github.com/apache/incubator-nuttx/commit/3adcae8ffbf247f1038bbb3b8281ea811799031e) because there were issues in passing a union with certain compilers (SDCC at the time).
>
> So, please don't consider using void * instead of wdparm_t. You will definitely break things!
And if FAR void * will break things, there are many basic components will break too, e.g.:
```
/* This struct defines the form of an interrupt service routine */
typedef CODE int (*xcpt_t)(int irq, FAR void *context, FAR void *arg);
/* Defines the work callback */
typedef CODE void (*worker_t)(FAR void *arg);
#ifndef __PTHREAD_ADDR_T_DEFINED
typedef FAR void *pthread_addr_t;
#define __PTHREAD_ADDR_T_DEFINED 1
#endif
typedef CODE pthread_addr_t (*pthread_startroutine_t)(pthread_addr_t);
typedef pthread_startroutine_t pthread_func_t;
/* Non-standard convenience definition of signal handling function types.
* These should be used only internally within the NuttX signal logic.
*/
typedef CODE void (*_sa_handler_t)(int signo);
typedef CODE void (*_sa_sigaction_t)(int signo, FAR siginfo_t *siginfo,
FAR void *context);
int on_exit(CODE void (*func)(int, FAR void *), FAR void *arg);
/* thrd_start_t: function pointer type passed to thrd_create */
typedef CODE int (*thrd_start_t)(FAR void *arg);
/* tss_dtor_t: function pointer type used for TSS destructor */
typedef CODE void (*tss_dtor_t)(FAR void *);
```
I don't think without irq/work especially irq, the system can work as expect.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672867706
> > But don't use type void *. There are problems using type void * with certain architctures (AVR I think) that was fixed by created wdparm_t. Please continue to use wdparm_t.
>
> The argument was originally a uint32_t. I was changed to a union with commit [b8f3bd8](https://github.com/apache/incubator-nuttx/commit/b8f3bd857be74ff78e01d961afbaedfbf17814ed) because there are many architectures where sizeof(void *) < sizeof(uint32_t), MCS51, ez80, etc. So void * is insufficient for passing a uint32_t.
>
Yes, `sizeof(void *)` may small than `sizeof(uint32_t)`, but I am wondering that is it possible that `sizeof(FAR void*) < sizeof(uint32_t)`.
> This was changed again to the current wdparm_t form with commit [3adcae8](https://github.com/apache/incubator-nuttx/commit/3adcae8ffbf247f1038bbb3b8281ea811799031e) because there were issues in passing a union with certain compilers (SDCC at the time).
>
> So, please don't consider using void * instead of wdparm_t. You will definitely break things!
And if `FAR void *` will break things, there are many basic components will break too, e.g.:
```
/* This struct defines the form of an interrupt service routine */
typedef CODE int (*xcpt_t)(int irq, FAR void *context, FAR void *arg);
/* Defines the work callback */
typedef CODE void (*worker_t)(FAR void *arg);
#ifndef __PTHREAD_ADDR_T_DEFINED
typedef FAR void *pthread_addr_t;
#define __PTHREAD_ADDR_T_DEFINED 1
#endif
typedef CODE pthread_addr_t (*pthread_startroutine_t)(pthread_addr_t);
typedef pthread_startroutine_t pthread_func_t;
/* Non-standard convenience definition of signal handling function types.
* These should be used only internally within the NuttX signal logic.
*/
typedef CODE void (*_sa_handler_t)(int signo);
typedef CODE void (*_sa_sigaction_t)(int signo, FAR siginfo_t *siginfo,
FAR void *context);
int on_exit(CODE void (*func)(int, FAR void *), FAR void *arg);
/* thrd_start_t: function pointer type passed to thrd_create */
typedef CODE int (*thrd_start_t)(FAR void *arg);
/* tss_dtor_t: function pointer type used for TSS destructor */
typedef CODE void (*tss_dtor_t)(FAR void *);
```
I don't think without irq/work especially irq, the system can work as expect.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672212866
The inefficiency in the proposal should be considered as well. You once told me that all calls to wd_start() but one provided only a single parameter. it sems wasteful to set up a structure containing that one parameter and passing it to to wd_start().
What is the size argument? There can be no more than CONFIG_WDOG_MAXPARMS arguments and each must be size wdparm_t. A size_t is used to hold the largest possible memory object (in bytes). A count of arguments makes more since and should be just an int.
So If I wanted to pass one argument I would have to do:
wdparm_t wdarg = (wdparm_t)arg;
ret = wd_start(&wdog, delay, entry, sizeof(wdarg), &wdarg);
Then you would have to divide size by sizeof(wdparm_t) to get the number of arguments?
Versus my suggestion:
ret = wd_start(&wdog, delay, entry, arg, 0);
Which looks less "ugly" to you. (We should not be using ugliness as a value for making engineering decisions unless all other things are equal. I think we say ugly because we can't verbal the reasons for preferring one thing over an another.)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672929712
> but can we assume that `sizeof(FAR void *)` is always equals to `sizeof(uintptr_t)` and `>= sizeof(uint32_t)`?
No. that is a false assumption. sizeof(uintptr) can be less than sizeof(uint32_t).. On most 8-bit machines, sizeof(uintptr_t) is or should be 2 bytes. On ez80 it is either 2 or 3 bytes depending on mode. For ez80, you can see:
In include/stdint.h:
/* Integer types capable of holding object pointers
* As a general rule, the size of size_t should be the same as the size of
* uintptr_t: 32-bits on a machine with 32-bit addressing but 64-bits on a
* machine with 64-bit addressing.
*/
typedef _ssize_t intptr_t;
typedef _size_t uintptr_t;
And in arch/z80/include/ez80/types.h:
/* These are the sizes of the standard integer types. NOTE that these type
* names have a leading underscore character. This file will be included
* (indirectly) by include/stdint.h and typedef'ed to the final name without
* the underscore character. This roundabout way of doings things allows
* the stdint.h to be removed from the include/ directory in the event that
* the user prefers to use the definitions provided by their toolchain header
* files
*
* These are the sizes of the types supported by the ZiLOG compiler:
*
* int - 24-bits
* short - 16-bits
* long - 32-bits
* char - 8-bits
* float - 32-bits
*/
...
/* A pointer is 2 or 3 bytes, depending upon if the ez80 is in z80
* compatibility mode or not
*
* Z80 mode - 16 bits
* ADL mode - 24 bits
*/
...
#elif defined(CONFIG_EZ80_Z80MODE)
typedef signed short _ssize_t;
typedef unsigned short _size_t;
#else
typedef signed int _ssize_t;
typedef unsigned int _size_t;
#endif
And in inttypes.h:
/* A pointer is 2 or 3 bytes, depending upon if the ez80 is in z80
* compatibility mode or not
*
* Z80 mode - 16 bits
* ADL mode - 24 bits
*/
#define PTR_MIN (-PTR_MAX - 1)
#ifdef CONFIG_EZ80_Z80MODE
#define PTR_MAX 32767
#define UPTR_MAX 65535U
#else
#define PTR_MAX 8388607
#define UPTR_MAX 16777215U
#endif
For AVR, the size of a pointer is different on the I-space bus and on the D-space bus. As I recall, sizeof(FAR void *) is usualy 16-bits but might be as much as 24-bits).
In fact sizeof(uinptr_t) < sizeof(uint32_t) is very common among the support architecures. You must not use FAR void * as a container for arbitrary values. You will break MANY architectures and we cannot permit you to do that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672890945
> And if `FAR void *` will break things, there are many basic components will break too
No, in general no. FAR void * is only a fatal decision if you intend to use it as a "container" to hold variables of other sizes. sizeof(uint32_t) is always 32 bits, but sizeof(void *) is variable on different MCUs from 16 to 64 bits.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672221606
@papatience after reviewing all caller of wd_start, all place just need one argument so, the prototype can be simple as:
```
typedef CODE void (*wdentry_t)(FAR void *arg);
```
just like what work_s done. If out of tree usage need pass more context info, he/she can always allocate the memory as need. So here is the patch: https://github.com/apache/incubator-nuttx/pull/1565
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672221606
@papatience after reviewing all caller of wd_start, all place just need one argument so, the prototype can be simple as:
```
typedef CODE void (*wdentry_t)(FAR void *arg);
```
just like what work_s done. If out of tree usage need pass more context info, he/she can always allocate the memory as need.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672937870
> Is it reasonable?
Absolutely NOT! wdparm_t was added to work around the kind of errors that you are tying to introduce. What you are proposing will work on most 32-bit machines but no where else and, hence, cannot be permitted. I will close an PR that attempts to pass watchdog paramters using FAR void * when the correct solution is already in place. What you are proposing is just wrong!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672937870
> Is it reasonable?
Absolutely NOT! wdparm_t was added to work around the kind of errors that you are tying to re-introduce. What you are proposing will work on most 32- and 64- bit machines but no where else and, hence, cannot be permitted. I will close an PR that attempts to pass watchdog paramters using FAR void * when the correct solution is already in place. What you are proposing is just wrong!
If a memory storage location is allocated as a FAR void * it will be 16, 20, 24, 32, or 64 bits wide and in many cases cannot hold a uint32_t. If it is allocated as a wdparm_t it will be 32 or 64 bits in width and is guaranteed to hold a uint32_t or a pointer in ALL cases. You proposal will not work and will break 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672937870
> Is it reasonable?
Absolutely NOT! wdparm_t was added to work around the kind of errors that you are tying to re-introduce. What you are proposing will work on most 32- and 64- bit machines but no where else and, hence, cannot be permitted. I will close an PR that attempts to pass watchdog paramters using FAR void * when the correct solution is already in place. What you are proposing is just wrong!
If a memory storage location is allocated as a FAR void * it will be 16, 20, 24, 32, or 64 bits wide and in many cases cannot hold a uint32_t. If it is allocated as a wdparm_t it will be 32 or 64 bits in width and is guaranteed to hold a uint32_t or a pointer in ALL cases. You proposal will not work and will break code.
This problem has already been resolved and the correct type is available, please don't be stubborn and harm the system.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672867706
> > But don't use type void *. There are problems using type void * with certain architctures (AVR I think) that was fixed by created wdparm_t. Please continue to use wdparm_t.
>
> The argument was originally a uint32_t. I was changed to a union with commit [b8f3bd8](https://github.com/apache/incubator-nuttx/commit/b8f3bd857be74ff78e01d961afbaedfbf17814ed) because there are many architectures where sizeof(void *) < sizeof(uint32_t), MCS51, ez80, etc. So void * is insufficient for passing a uint32_t.
>
Yes, `sizeof(void *)` may small than `sizeof(uint32_t)`, but I am wondering that is it possible that `sizeof(FAR void*) < sizeof(uint32_t)`.
> This was changed again to the current wdparm_t form with commit [3adcae8](https://github.com/apache/incubator-nuttx/commit/3adcae8ffbf247f1038bbb3b8281ea811799031e) because there were issues in passing a union with certain compilers (SDCC at the time).
>
> So, please don't consider using void * instead of wdparm_t. You will definitely break things!
And if FAR void * will break things, there are many basic components will break too, e.g.:
```
/* This struct defines the form of an interrupt service routine */
typedef CODE int (*xcpt_t)(int irq, FAR void *context, FAR void *arg);
/* Defines the work callback */
typedef CODE void (*worker_t)(FAR void *arg);
#ifndef __PTHREAD_ADDR_T_DEFINED
typedef FAR void *pthread_addr_t;
#define __PTHREAD_ADDR_T_DEFINED 1
#endif
typedef CODE pthread_addr_t (*pthread_startroutine_t)(pthread_addr_t);
typedef pthread_startroutine_t pthread_func_t;
/* Non-standard convenience definition of signal handling function types.
* These should be used only internally within the NuttX signal logic.
*/
typedef CODE void (*_sa_handler_t)(int signo);
typedef CODE void (*_sa_sigaction_t)(int signo, FAR siginfo_t *siginfo,
FAR void *context);
int on_exit(CODE void (*func)(int, FAR void *), FAR void *arg);
/* thrd_start_t: function pointer type passed to thrd_create */
typedef CODE int (*thrd_start_t)(FAR void *arg);
/* tss_dtor_t: function pointer type used for TSS destructor */
typedef CODE void (*tss_dtor_t)(FAR void *);
```
I don't think without irq/work especially irq, the system can work as expect.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #740: problem with nxsig_timeout (sig_timedwait.c)
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #740:
URL: https://github.com/apache/incubator-nuttx/issues/740#issuecomment-672247919
>
>
> @papatience after reviewing all caller of wd_start, all place just need one argument so, the prototype can be simple as:
>
> ```
> typedef CODE void (*wdentry_t)(FAR void *arg);
> ```
>
> just like what work_s done. If out of tree usage need pass more context info, he/she can always allocate the memory as need. So here is the patch: #1565
If arg is the actual, single argument and NOT a pointer to an array of arguments, then this seems okay. But don't use type void *. There are problems using type void * with certain architctures (AVR I think) that was fixed by created wdparm_t. Please continue to use wdparm_t.
Again, I think it is bad engineering judgement to make this change. Nothing is broken, this fixes nothing, and just adds turmoil and additional incompatibility to the documented OS internal interfaces. This is NOT responsible engineering.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org