[PATCH] profile support

Christopher Faylor cgf-use-the-mailinglist-please@cygwin.com
Mon Aug 29 20:24:00 GMT 2011


On Tue, Aug 30, 2011 at 05:10:39AM +0900, jojelino wrote:
>On 2011-08-23 PM 2:40, Christopher Faylor wrote:
>> On Tue, Aug 23, 2011 at 02:05:06PM +0900, jojelino wrote:
>>> Index: winsup/cygwin/Makefile.in
>>> ===================================================================
>>> RCS file: /cvs/src/src/winsup/cygwin/Makefile.in,v
>>> retrieving revision 1.248
>>> diff -u -p -r1.248 Makefile.in
>>> --- winsup/cygwin/Makefile.in	2 May 2011 19:14:39 -0000	1.248
>>> +++ winsup/cygwin/Makefile.in	22 Aug 2011 20:27:57 -0000
>>> @@ -233,7 +233,7 @@ EXTRALIBS:=libautomode.a libbinmode.a li
>>> INSTOBJS:=automode.o binmode.o textmode.o textreadmode.o
>>> TARGET_LIBS:=$(LIB_NAME) $(CYGWIN_START) $(GMON_START) $(LIBGMON_A) $(SUBLIBS) $(INSTOBJS) $(EXTRALIBS)
>>>
>>> -ifneq "${filter -O%,$(CFLAGS)}" ""
>>> +ifneq "" ""
>>>
>>> -    void *rv = malloc(size);
>>> +    void *rv = LocalAlloc(0x40,size);
>>
>> There are a few things in your patch which make no sense.  The above are
>> two of them.  I am not going to look further.  The patch certainly can't
>> go in as is.
>>
>> cgf
>>
>The two of them is now
>
>  ifneq "${filter -O%,$(CFLAGS)}" ""
>+ifneq '$(profile)' '1'

That would be an 'ifdef'.

>  endif
>...
>...
>+endif
>
>+#if !defined(PROFILE)
>      void *rv = malloc(size);
>+#else
>+    void *rv = LocalAlloc(LMEM_FIXED,size);
>+#endif

Since the code is in gmon.c then I don't see a reason to #ifdef it but I
still don't understand the motivation for the change.  This is a major
amount of code and it is desperately missing comments.

Maybe Corinna will disagree but I think there is way too much code
change here for me to be comfortable with including it.  It looks like
it would be an ongoing maintenance issue, requiring constant vigilance
to avoid code rot.  And, it would have to be very carefully studied to
make sure there aren't more gotchas like 'if "" ""'.

Sorry, but I don't see us including this.

cgf



More information about the Cygwin-patches mailing list