Re: RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-11 Thread Staffan Larsen
> On 11 dec. 2015, at 11:54, Dmitry Samersoff > wrote: > > Staffan, > >> This is all platform specific code - and all the platforms are >> different. Of course, it would be good if all platforms reported good >> error message always - and we can continue working on that. > > OK. > >> It shou

Re: RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-11 Thread Dmitry Samersoff
Staffan, > This is all platform specific code - and all the platforms are > different. Of course, it would be good if all platforms reported good > error message always - and we can continue working on that. OK. > It should get filled in in the ptrace_attach() call. if ph->pid == thr->lwp_id at

Re: RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-11 Thread Staffan Larsen
> On 10 dec. 2015, at 23:20, Dmitry Samersoff > wrote: > > Staffan, > > 1. > > strerror_r comes in two versions - returning string (GNU) and returning > int (XSI) > > To make developers live more interesting GNU version doesn't guarantee > to fill buf with appropriate string, so you can't ju

Re: RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-10 Thread Thomas Stüfe
Hi Dmitry, > Or just don't care of threads and use plain strerror - it widely used in > hotspot and it doesn't crash on a race. > > Unfortunately, it does: https://bugs.openjdk.java.net/browse/JDK-8133249 Regards, Thomas > >

Re: RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-10 Thread Dmitry Samersoff
Staffan, 1. strerror_r comes in two versions - returning string (GNU) and returning int (XSI) To make developers live more interesting GNU version doesn't guarantee to fill buf with appropriate string, so you can't just void return value. Default version tends to be XSI now days and I'm not sur

Re: RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-10 Thread Staffan Larsen
Thanks! > On 10 dec. 2015, at 15:26, Jaroslav Bachorik > wrote: > > On 10.12.2015 15:22, Staffan Larsen wrote: >> Hi Thommas, >> >>> On 10 dec. 2015, at 14:49, Thomas Stüfe >> > wrote: >>> >>> Hi Stefan, >>> >>> Disclaimer: not a "R"eviewer. >>> >>> http://cr

Re: RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-10 Thread Staffan Larsen
Thanks! > On 10 dec. 2015, at 15:56, Thomas Stüfe wrote: > > Hi Staffan, > > this looks fine now, thanks! > > ..Thomas > > On Thu, Dec 10, 2015 at 3:42 PM, Staffan Larsen > wrote: > Hi Thomas, > >> On 10 dec. 2015, at 15:27, Thomas Stüfe >

Re: RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-10 Thread Thomas Stüfe
Hi Staffan, this looks fine now, thanks! ..Thomas On Thu, Dec 10, 2015 at 3:42 PM, Staffan Larsen wrote: > Hi Thomas, > > On 10 dec. 2015, at 15:27, Thomas Stüfe wrote: > > Hi Staffan, > > thanks! > > It also just occurred to me that strerror is not considered threadsafe. > Maybe strerror_r()

Re: RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-10 Thread Staffan Larsen
Hi Thomas, > On 10 dec. 2015, at 15:27, Thomas Stüfe wrote: > > Hi Staffan, > > thanks! > > It also just occurred to me that strerror is not considered threadsafe. Maybe > strerror_r() would a better option (depending on the version you use you > would have to check the return value for NUL

Re: RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-10 Thread Thomas Stüfe
Hi Staffan, thanks! It also just occurred to me that strerror is not considered threadsafe. Maybe strerror_r() would a better option (depending on the version you use you would have to check the return value for NULL, though). sorry for this piecemeal review. Kind Regards, Thomas On Thu, Dec

Re: RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-10 Thread Jaroslav Bachorik
On 10.12.2015 15:22, Staffan Larsen wrote: Hi Thommas, On 10 dec. 2015, at 14:49, Thomas Stüfe mailto:thomas.stu...@gmail.com>> wrote: Hi Stefan, Disclaimer: not a "R"eviewer. http://cr.openjdk.java.net/~sla/JDK-8145099/webrev.01/agent/src/os/linux/LinuxDebuggerLocal.c.udiff.html I am not s

Re: RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-10 Thread Staffan Larsen
Hi Thommas, > On 10 dec. 2015, at 14:49, Thomas Stüfe wrote: > > Hi Stefan, > > Disclaimer: not a "R"eviewer. > > http://cr.openjdk.java.net/~sla/JDK-8145099/webrev.01/agent/src/os/linux/LinuxDebuggerLocal.c.udiff.html > >

Re: RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-10 Thread Thomas Stüfe
Hi Stefan, Disclaimer: not a "R"eviewer. http://cr.openjdk.java.net/~sla/JDK-8145099/webrev.01/agent/src/os/linux/LinuxDebuggerLocal.c.udiff.html I am not sure why you pass sizeof(err_buf) - 1 instead of sizeof(err_buf) to Pgrab and sizeof(msg) - 1 instead of sizeof(msg) to snprintf? snprintf wi

RFR: JDK-8145099 Better error message when SA can't attach to a process

2015-12-10 Thread Staffan Larsen
Please review this patch to add a better error message to SA when it fails to attach to a process on linux. Currently the error says "Can't attach to the process”. After this change the message will look something like: "Can't attach to the process: ptrace(PTRACE_ATTACH, ..) failed for 28417: Op