tech-userlevel archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: _SC_SIGQUEUE_MAX



Hi Christos,

    Please find my comments below.

2016-06-14 9:50 GMT-07:00 Christos Zoulas <christos%astron.com@localhost>:

> In article <
> CA+SXE9s4bY1KLXgXUWWgdv+0_Z-baLpBg2WoEG18TfyG1YWfDg%mail.gmail.com@localhost>,
> Charles Cui  <charles.cui1984%gmail.com@localhost> wrote:
> >
> >Let me know if there are problems. In the next several days, I will try to
> >add some unit tests.
> >After adding the unit tests, I will continue the work of real time
> signals.
>
> They mostly look fine. I am not sure that stuff has ever been tested
> this is why we need to add unit tests. I am not going to commit
> them before we make sure that they work properly, and for that we
> need real unit-tests.  For example, I had asked you to change the
> hard-coded masks to macros... One of the reasons I did that is
> because the code looks wrong, for example:
>
>  pthread_mutexattr_gettype(const pthread_mutexattr_t *attr, int *typep)
>  {
> +       uintptr_t val;
> +
>         pthread__error(EINVAL, "Invalid mutex attribute",
>             attr->ptma_magic == _PT_MUTEXATTR_MAGIC);
>
> -       *typep = (int)(intptr_t)attr->ptma_private;
> +       val = (uintptr_t)attr->ptma_private & ~0x00ffL;
> +       *typep = (int)val;
>         return 0;
>  }
>
>
> That should be & 0x00ff probably. Don't bother fixing it now, I have
> created macros for all of them and fixed some other minor indentation
> and whitespace nits. I will post the diffs once everything builds later.
>
I am sorry to miss your instruction of using macro instead of direct bit
mask.
and Thanks for your work!

>
> There are a few questions:
>
> 1. Should we commit the pshared stuff now (when we don't really support
>    process shared mutexes), or will that confuse 3rd party software?

I think we can hold the pshared stuff until we have cross process
synchronization.
You can commit other man pages.

>

2. Should there be more technical review for ad@'s priority inversion work?
>
Yes, we should. We need more persons with deep understanding of NetBSD
(like you and Martin) to review
the patch before we commit because it is not a small feature.

> 3. Is the PTHREAD_STACK_MIN patch useful, or can we do better?
>
This patch actually makes use of MINSIGSTKSZ, which is a fixed value in
NetBSD and is arch dependent
in FreeBSD (the accurate name is __MINSIGSTKSZ). So, we may go to the
FreeBSD's way to make
the macro arch dependent if there is no netbsd specific reason to keep it
as a const.


So to conclude, the action items for me is to
1. add unit tests for ad's work to make sure it works well.
2. may add some changes according to your comments on PTHREAD_STACK_MIN.

>
>
> Best,
>
> christos
>
>
>


Home | Main Index | Thread Index | Old Index