[PATCH] Support profiling of multi-threaded apps.

Corinna Vinschen corinna-cygwin@cygwin.com
Thu Mar 10 08:48:00 GMT 2016


On Mar  9 16:54, Mark Geisert wrote:
> On Wed, 9 Mar 2016, Corinna Vinschen wrote:
> >Hi Mark,
> >
> >On Mar  9 00:39, Mark Geisert wrote:
> >>This is Version 3 incorporating review comments of Version 2.  This is just
> >>the code patch; a separate doc patch is forthcoming.
> >
> >The patch looks fine to me code-wise.  I just have a few style requests:
> >
> >>+	if ((prefix = getenv("GMON_OUT_PREFIX")) != NULL) {
> >>+		char *buf;
> >>+		long divisor = 1000*1000*1000;	// covers positive pid_t values
> >
> >Why "long"?  It's safe to use here, but it doesn't match the incoming
> >datatype.  pid_t is 4 bytes, but long is 8 bytes on x86_64.  If you
> >like it better that way we can keep it in but wouldn't, say, int32_t
> >be a better match?  Also, can you convert the TAB to a space preceeding
> >the comment so it's within 80 columns, please?
> 
> The "long" was a dumb mistake (and malingering 32-bit orientation) on my
> part.  It shall be made int32_t of course.
> 
> >I'm also a bit unhappy with some of the comments not trailing on a code
> >line.  E.g.:
> >
> >// sample the pc of the thread indicated by thr, but bail if anything amiss
> >
> >// record profiling samples for other pthreads, if any
> >
> >Ideally that would be /**/ style comments, starting in uppercase and
> >as full sentences ending with a dot, e.g.:
> >
> >/* Sample the pc of the thread indicated by thr, but bail if anything amiss. */
> >
> >/* Record profiling samples for other pthreads, if any. */
> 
> I'll go over the whole patch set to get rid of nonleading TABs and to fix
> the comments as you suggest.  The latter was just me not realizing there was
> a convention to follow so I didn't :-).  All shall be fixed.

Thanks.

> gmon.c/.h need to be reformatted to GNU style like the rest of Cygwin but
> I'm leaving that to a future patch (NO I AM NOT COMMITTING TO THAT!! :-).

Haha!  But, seriosly, indent(1) is a wonderful tool for that.  With the
right options there's no subsequent manual work.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://cygwin.com/pipermail/cygwin-patches/attachments/20160310/eb5c7a59/attachment.sig>


More information about the Cygwin-patches mailing list