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.
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!! :-).
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
Cheers,
..mark
