Re: [PATCH] tracepoint.c

Re: [PATCH] tracepoint.c

[Klee Dienes]

> Ah, I think we've had a communication breakdown.  I thought that
> your 
> previous patch was defunct, and we were waiting for you to resubmit
> it. 
> Rereading the old thread, I can see where I lost continuity.  Sorry
> for 
> the confusion -- can we start again? 
>  
> I actually liked the 'save-breakpoints' command, and was thinking 
> of pinging you to see when you planned to resubmit it.  But I don't 
> like it being grouped together with the 'future-break' command. 
> They're really separate, though related, and I'd rather consider 
> separate functionalities separately.  Besides, the two together 
> make a really huge patch, one that it's difficult to review  
> line by line. 
 
OK, that's fair.  My main reason for combining the patches was that 
they had mutual dependencies on each other ('save-breakpoints' knows 
about 'future' breakpoints so that it can save and restore them; the 
future-break code knows about the 'original-flags' field added by 
'save-breakpoints').  But I can probably remove the future-break 
support from the 'save-breakpoints' command, and resubmit future-break 
once 'save-breakpoints' is committed. 
 
> As for the change to tracepoints, I had that sitting in my source 
> tree from your earlier submission, and I was just cleaning up loose 
> ends.  I decided to make sure that didn't get lost, while waiting 
> for you to resubmit your patch.  Sorry if I jumped the gun on you. 
 
Anything that reduces the size of our diffs is a win from my 
perspective; I just wanted to make sure I was understanding the 
process properly. 
 
> If a week goes by without a response, you should ping the list. 
> We might have gotten distracted ourselves, or there could be a 
> misunderstanding such as this one. 
 
OK, will do.  I believe there's only one other patch outstanding at 
this point; I'm just eager to get it resolved, since it's holding up 
some of our more interesting Objective-C patches.  I'll send a ping 
now; thanks for the advice! 

[PATCH] tracepoint.c

[Michael Snyder]

2002-01-07  Michael Snyder  <msnyder@redhat.com>

	* tracepoint.c (tracepoint_save_command): From Klee Dienes --
	use tilde_expand and strerror for opening save-tracepoints file.

Index: tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.c,v
retrieving revision 1.29
retrieving revision 1.30
diff -p -r1.29 -r1.30
*** tracepoint.c	2001/12/25 22:24:38	1.29
--- tracepoint.c	2002/01/08 01:52:12	1.30
*************** tracepoint_save_command (char *args, int
*** 2263,2269 ****
    struct action_line *line;
    FILE *fp;
    char *i1 = "    ", *i2 = "      ";
!   char *indent, *actionline;
    char tmp[40];
  
    if (args == 0 || *args == 0)
--- 2263,2269 ----
    struct action_line *line;
    FILE *fp;
    char *i1 = "    ", *i2 = "      ";
!   char *indent, *actionline, *pathname;
    char tmp[40];
  
    if (args == 0 || *args == 0)
*************** tracepoint_save_command (char *args, int
*** 2275,2283 ****
        return;
      }
  
!   if (!(fp = fopen (args, "w")))
!     error ("Unable to open file '%s' for saving tracepoints");
! 
    ALL_TRACEPOINTS (tp)
    {
      if (tp->addr_string)
--- 2275,2286 ----
        return;
      }
  
!   pathname = tilde_expand (args);
!   if (!(fp = fopen (pathname, "w")))
!     error ("Unable to open file '%s' for saving tracepoints (%s)",
! 	   args, strerror (errno));
!   xfree (pathname);
!   
    ALL_TRACEPOINTS (tp)
    {
      if (tp->addr_string)

Re: [PATCH] tracepoint.c

[Klee Dienes]

Michael Snyder <msnyder@cygnus.com> writes:

> 2002-01-07  Michael Snyder  <msnyder@redhat.com>
> 
> 	* tracepoint.c (tracepoint_save_command): From Klee Dienes --
> 	use tilde_expand and strerror for opening save-tracepoints file.
> 

Just to make sure I understand the procedure I should be following:

Does this mean that I should re-submit a version of the
'save-breakpoints' patch with this change removed from it?  Or does it
just mean that this part of the patch has been accepted, and I should
wait to hear from the other relevant maintainers before revising or
committing the rest of the save-breakpoint patch?  If the latter, why
not just say "the changes to tracepoint.c are approved; please commit
them"?  I don't mean this as complaint, just trying to make sure I'm
following the system properly. 

Also, is there a formalized way to ping or somehow track already
submitted patches?  We've got a number of other patches pretty much
ready to submit (the Objective-C patches being the most notable of
these), but since they depend on some of the patches already
submitted, I was hoping to get these resolved first.  Should I just go
ahead and post them, with a note that they assume that some of the
already-submitted patches have been committed?  This can get to be a
real mess eventually, as when I modify one patch in response to
feedback, I then have to go modify all the dependent patches.  Or
should I just badger individual maintainers until the ones already
submitted have been resolved?

I realize the irony of showing up after years of code-divergence, and
then being in a big "rush rush rush" mode to get patches considered
for acceptance.  But I'm sure we (the Apple GDB engineers) only have a
limited window of oppotunity before some other crisis comes up to
distract us, and I'm hoping to take as much advantage of this
opportunity to merge the sources as I possibly can.

Thanks,
 - Klee

Re: [PATCH] tracepoint.c

[Michael Snyder]

Klee Dienes wrote:
> 
> Michael Snyder <msnyder@cygnus.com> writes:
> 
> > 2002-01-07  Michael Snyder  <msnyder@redhat.com>
> >
> >       * tracepoint.c (tracepoint_save_command): From Klee Dienes --
> >       use tilde_expand and strerror for opening save-tracepoints file.
> >
> 
> Just to make sure I understand the procedure I should be following:
> 
> Does this mean that I should re-submit a version of the
> 'save-breakpoints' patch with this change removed from it?  Or does it
> just mean that this part of the patch has been accepted, and I should
> wait to hear from the other relevant maintainers before revising or
> committing the rest of the save-breakpoint patch?  If the latter, why
> not just say "the changes to tracepoint.c are approved; please commit
> them"?  I don't mean this as complaint, just trying to make sure I'm
> following the system properly.

Ah, I think we've had a communication breakdown.  I thought that your
previous patch was defunct, and we were waiting for you to resubmit it.
Rereading the old thread, I can see where I lost continuity.  Sorry for
the confusion -- can we start again?

I actually liked the 'save-breakpoints' command, and was thinking
of pinging you to see when you planned to resubmit it.  But I don't
like it being grouped together with the 'future-break' command.
They're really separate, though related, and I'd rather consider
separate functionalities separately.  Besides, the two together
make a really huge patch, one that it's difficult to review 
line by line.

As for the change to tracepoints, I had that sitting in my source
tree from your earlier submission, and I was just cleaning up loose
ends.  I decided to make sure that didn't get lost, while waiting
for you to resubmit your patch.  Sorry if I jumped the gun on you.

> Also, is there a formalized way to ping or somehow track already
> submitted patches?  We've got a number of other patches pretty much
> ready to submit (the Objective-C patches being the most notable of
> these), but since they depend on some of the patches already
> submitted, I was hoping to get these resolved first.  Should I just go
> ahead and post them, with a note that they assume that some of the
> already-submitted patches have been committed?  This can get to be a
> real mess eventually, as when I modify one patch in response to
> feedback, I then have to go modify all the dependent patches.  Or
> should I just badger individual maintainers until the ones already
> submitted have been resolved?

Patches shouldn't take as long to be approved as this one.
I regret that your first major effort ran into a snag, and
promise to try and see that it doesn't happen again.  Having
multiple outstanding patches that depend on each other can be
a major headache.  I recommend making each patch as small and
discrete as possible, so that they can be approved quickly.


> I realize the irony of showing up after years of code-divergence, and
> then being in a big "rush rush rush" mode to get patches considered
> for acceptance.  But I'm sure we (the Apple GDB engineers) only have a
> limited window of oppotunity before some other crisis comes up to
> distract us, and I'm hoping to take as much advantage of this
> opportunity to merge the sources as I possibly can.

If a week goes by without a response, you should ping the list.
We might have gotten distracted ourselves, or there could be a
misunderstanding such as this one.