On 05/07/2012 10:55 AM, David Holmes wrote:
On 7/05/2012 5:21 PM, Rickard Bäckman wrote:
David,

I can't find that code in any of the modified files (only as "blue" text
in osThread_bsd.hpp, but that is removed code).

Sorry the frames view of the webrev confused me. It is "blue" but that
is normally added or modified code, while "red" is deleted. In this case
it seems it has been modified out of existence ;-)


It's the best kind of modification :)

/R

David

There is a set_thread_id in osThread.hpp

118 void set_thread_id(thread_id_t id) { _thread_id = id; }

Thanks
/R

On 05/06/2012 02:10 PM, David Holmes wrote:
Hi Rickard,

One initial comment. Isn't this redundant:

102 #ifdef _ALLBSD_SOURCE
103 #ifdef __APPLE__
104 void set_thread_id(thread_t id) {
105 _thread_id = id;
106 }
107 #else
108 void set_thread_id(pthread_t id) {
109 _thread_id = id;
110 }
111 #endif
112 #else
113 void set_thread_id(pid_t id) {
114 _thread_id = id;
115 }
116 #endif

Given you have set a typedef for thread_id_t, and rely on it to avoid a
cast on the assignment, can't you just define a single function:

void set_thread_id(thread_id_t id) {
_thread_id = id;
}

?

David
-----

On 3/05/2012 7:39 PM, Rickard Bäckman wrote:
Hi all,

I've made a refactoring of thread_id in OSThread, which reduces the
amount of duplicated code.

Please review this change at:
http://cr.openjdk.java.net/~rbackman/7161732/webrev/

Thanks
/R


Reply via email to