Hi Staffan, this looks fine now, thanks!
..Thomas On Thu, Dec 10, 2015 at 3:42 PM, Staffan Larsen <staffan.lar...@oracle.com> wrote: > Hi Thomas, > > On 10 dec. 2015, at 15:27, Thomas Stüfe <thomas.stu...@gmail.com> 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 NULL, though). > > > Updated here: http://cr.openjdk.java.net/~sla/JDK-8145099/webrev.03/ > > Thanks, > /Staffan > > > sorry for this piecemeal review. > > Kind Regards, Thomas > > > > On Thu, Dec 10, 2015 at 3:22 PM, Staffan Larsen <staffan.lar...@oracle.com > > wrote: > >> Hi Thommas, >> >> On 10 dec. 2015, at 14:49, Thomas Stüfe <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 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 >> will always zero terminate in case of truncation, at least on posix >> platforms. >> >> >> Good point. I was just being overly cautious without thinking. >> >> Updated webrev: http://cr.openjdk.java.net/~sla/JDK-8145099/webrev.02/ >> >> Thanks, >> /Staffan >> >> >> Otherwise this looks good. >> >> Kind Regards, Thomas >> >> >> On Thu, Dec 10, 2015 at 2:19 PM, Staffan Larsen < >> staffan.lar...@oracle.com> wrote: >> >>> 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: Operation not permitted” >>> >>> webrev: http://cr.openjdk.java.net/~sla/JDK-8145099/webrev.01/ >>> bug: https://bugs.openjdk.java.net/browse/JDK-8145099 >>> >>> Thanks, >>> /Staffan >>> >>> >>> >>> >> >> > >
