On Fri, Sep 30, 2016 at 2:18 PM, Justin Cinkelj <justin.cink...@xlab.si>
wrote:

> Nadav, can you accept the v4 patch? I think I took into account your
> comments - if not, I will update and send v5 patch.
>

Thanks Justin, the patch looks good now. However, I'm still not thrilled
about adding "C" versions of random parts of OSv's C++ API. This is a
slippery slope, because there's so much that is missing a C API and could
be added. Moreover, even C applications can easily use C++ code -
basically, add one C++ source file to the application - even the same
source file you proposed we add to OSv.

However, if adding this into OSv simplifies your life (maybe you can't
modify OpenMPI's makefile or something?) please let me know and I'll bit
the bullet, and add this patch to OSv.


>
> BR Justin
>
>
> On 07/05/2016 09:23 AM, Justin Cinkelj wrote:
>
>
>
> On 07/05/2016 09:12 AM, Nadav Har'El wrote:
>
> On Tue, Jul 5, 2016 at 9:56 AM, Justin Cinkelj <justin.cink...@xlab.si>
> wrote:
>
>> Signed-off-by: Justin Cinkelj <justin.cink...@xlab.si>
>> ---
>>  Makefile                     |  1 +
>>  core/osv_c_wrappers.cc       | 33 +++++++++++++++++++++++++++++++++
>>  include/osv/osv_c_wrappers.h | 27 +++++++++++++++++++++++++++
>>  3 files changed, 61 insertions(+)
>>  create mode 100644 core/osv_c_wrappers.cc
>>  create mode 100644 include/osv/osv_c_wrappers.h
>>
>> diff --git a/Makefile b/Makefile
>> index 7e15d0e..95da609 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -908,6 +908,7 @@ objects += core/net_trace.o
>>  objects += core/app.o
>>  objects += core/libaio.o
>>  objects += core/osv_execve.o
>> +objects += core/osv_c_wrappers.o
>>
>>  #include $(src)/libc/build.mk:
>>  libc =
>> diff --git a/core/osv_c_wrappers.cc b/core/osv_c_wrappers.cc
>> new file mode 100644
>> index 0000000..abd5fe3
>> --- /dev/null
>> +++ b/core/osv_c_wrappers.cc
>> @@ -0,0 +1,33 @@
>> +
>> +#include <osv/debug.hh>
>> +#include <osv/sched.hh>
>> +#include <osv/app.hh>
>> +
>> +using namespace osv;
>>
>
> Are you actually using anything from the "osv" namespace? (maybe you are,
> I just didn't notice).
>
> only with_all_app_threads.
>
>
>
>> +using namespace sched;
>> +
>> +extern "C" {
>>
>
> If you #include osv_c_wrappers.h (which you should), you won't need this
> "extern C" because it would be already in the header file.
>
> Ah... makes sense now.
> Thank you.
> Justin
>
>
>
>> +
>> +int osv_get_all_app_threads(pid_t tid, pid_t** tid_arr, size_t *len) {
>> +    thread* app_thread = tid==0? thread::current():
>> thread::find_by_id(tid);
>> +    if (app_thread == nullptr) {
>> +        return ESRCH;
>> +    }
>> +    std::vector<thread*> app_threads;
>> +    with_all_app_threads([&](thread& th2) {
>> +        app_threads.push_back(&th2);
>> +    }, *app_thread);
>> +
>> +    *tid_arr = (pid_t*)malloc(app_threads.size()*sizeof(pid_t));
>> +    if (*tid_arr == nullptr) {
>> +        *len = 0;
>> +        return ENOMEM;
>> +    }
>> +    *len = 0;
>> +    for (auto th : app_threads) {
>> +        (*tid_arr)[(*len)++] = th->id();
>> +    }
>> +    return 0;
>> +}
>> +
>> +}
>> diff --git a/include/osv/osv_c_wrappers.h b/include/osv/osv_c_wrappers.h
>> new file mode 100644
>> index 0000000..acef016
>> --- /dev/null
>> +++ b/include/osv/osv_c_wrappers.h
>> @@ -0,0 +1,27 @@
>> +/*
>> + * Copyright (C) 2016 XLAB, d.o.o.
>> + *
>> + * This work is open source software, licensed under the terms of the
>> + * BSD license as described in the LICENSE file in the top-level
>> directory.
>> + */
>> +
>> +#ifndef INCLUDED_OSV_C_WRAPPERS_H
>> +#define INCLUDED_OSV_C_WRAPPERS_H
>> +
>> +#ifdef __cplusplus
>> +extern "C" {
>> +#endif
>> +
>> +/*
>> +Save in *tid_arr array TIDs of all threads from app which "owns" input
>> tid/thread.
>> +*tid_arr is allocated with malloc, *len holds lenght.
>> +Caller is responsible to free tid_arr.
>> +Returns 0 on success, error code on error.
>> +*/
>> +int osv_get_all_app_threads(pid_t tid, pid_t** tid_arr, size_t* max_len);
>> +
>> +#ifdef __cplusplus
>> +}
>> +#endif
>> +
>> +#endif /* INCLUDED_OSV_C_WRAPPERS_H */
>> --
>> 2.5.0
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "OSv Development" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to osv-dev+unsubscr...@googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.
>>
>
>
>
>

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to