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 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 >> >> >> >> > >
