This is the mail archive of the cygwin-patches mailing list for the Cygwin project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
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
Attachment:
signature.asc
Description: PGP signature
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |