[PATCH] Support profiling of multi-threaded apps.

Corinna Vinschen corinna-cygwin@cygwin.com
Wed Mar 9 22:44:00 GMT 2016


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?

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. */

With these changes I'll apply the patch.


Thanks,
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/20160309/adff4bf0/attachment.sig>


More information about the Cygwin-patches mailing list