Unbreak DJGPP port of GDB

Message ID 83zj4uypea.fsf@gnu.org
State New, archived
Headers

Commit Message

Eli Zaretskii May 24, 2015, 3:19 p.m. UTC
  It turns out the DJGPP (a.k.a. "go32") port of GDB was broken more
than a year ago, by commits 9b40951 and bd265cd.  It stayed broken
ever since.  Lately, Andris Pavenis bisected the bug and posted the
results here:

  http://www.delorie.com/djgpp/mail-archives/browse.cgi?p=djgpp/2015/05/24/04:17:40

Looking into those changesets, I've found that the problem was due to
incorrect handling of values returned by functions 'read_child' and
'write_child', which are part of the DJGPP native debug support.

The patch below fixes that.  I will push it in a few days (with an
appropriate ChangeLog entry), if no one objects.

Thanks.
  

Comments

Yao Qi May 28, 2015, 8:46 a.m. UTC | #1
Eli Zaretskii <eliz@gnu.org> writes:

> It turns out the DJGPP (a.k.a. "go32") port of GDB was broken more
> than a year ago, by commits 9b40951 and bd265cd.  It stayed broken
> ever since.  Lately, Andris Pavenis bisected the bug and posted the
> results here:
>
>   http://www.delorie.com/djgpp/mail-archives/browse.cgi?p=djgpp/2015/05/24/04:17:40
>
> Looking into those changesets, I've found that the problem was due to
> incorrect handling of values returned by functions 'read_child' and
> 'write_child', which are part of the DJGPP native debug support.

It is broken by my to_xfer_partial patches... sorry about that.

>
> The patch below fixes that.  I will push it in a few days (with an
> appropriate ChangeLog entry), if no one objects.

The patch looks right to me.
  
Eli Zaretskii May 28, 2015, 2:41 p.m. UTC | #2
> From: Yao Qi <qiyaoltc@gmail.com>
> Cc: gdb-patches@sourceware.org
> Date: Thu, 28 May 2015 09:46:59 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > It turns out the DJGPP (a.k.a. "go32") port of GDB was broken more
> > than a year ago, by commits 9b40951 and bd265cd.  It stayed broken
> > ever since.  Lately, Andris Pavenis bisected the bug and posted the
> > results here:
> >
> >   http://www.delorie.com/djgpp/mail-archives/browse.cgi?p=djgpp/2015/05/24/04:17:40
> >
> > Looking into those changesets, I've found that the problem was due to
> > incorrect handling of values returned by functions 'read_child' and
> > 'write_child', which are part of the DJGPP native debug support.
> 
> It is broken by my to_xfer_partial patches... sorry about that.

No sweat.  I should have been more vigilant in reviewing those patches
anyway.

> > The patch below fixes that.  I will push it in a few days (with an
> > appropriate ChangeLog entry), if no one objects.
> 
> The patch looks right to me.

Thanks.
  
Eli Zaretskii May 30, 2015, 10:11 a.m. UTC | #3
> Date: Thu, 28 May 2015 17:41:56 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: gdb-patches@sourceware.org
> 
> > From: Yao Qi <qiyaoltc@gmail.com>
> > Cc: gdb-patches@sourceware.org
> > Date: Thu, 28 May 2015 09:46:59 +0100
> > 
> > Eli Zaretskii <eliz@gnu.org> writes:
> > 
> > > It turns out the DJGPP (a.k.a. "go32") port of GDB was broken more
> > > than a year ago, by commits 9b40951 and bd265cd.  It stayed broken
> > > ever since.  Lately, Andris Pavenis bisected the bug and posted the
> > > results here:
> > >
> > >   http://www.delorie.com/djgpp/mail-archives/browse.cgi?p=djgpp/2015/05/24/04:17:40
> > >
> > > Looking into those changesets, I've found that the problem was due to
> > > incorrect handling of values returned by functions 'read_child' and
> > > 'write_child', which are part of the DJGPP native debug support.
> > 
> > It is broken by my to_xfer_partial patches... sorry about that.
> 
> No sweat.  I should have been more vigilant in reviewing those patches
> anyway.
> 
> > > The patch below fixes that.  I will push it in a few days (with an
> > > appropriate ChangeLog entry), if no one objects.
> > 
> > The patch looks right to me.
> 
> Thanks.

No further comments, so I pushed the changeset, bot to master and to
the 7.9 branch.
  
Joel Brobecker June 3, 2015, 10:01 p.m. UTC | #4
> No further comments, so I pushed the changeset, bot to master and to
> the 7.9 branch.

I didn't think we would be making any new release out of the 7.9 branch,
but maybe this is grounds for yet another one. Personally, if it was
missed by during pre-release, release, and then corrective release,
perhaps it's OK to wait for 7.10 instead, which is probably about
4-6 weeks away. But, it DJGPP users tell me it's going to be make
difference, I'm open to the idea of spending a couple of hours making
a 7.9.2.

Regardless, the procedure when pushing changes to the branch after
a release has been made is to update the wiki:

    https://sourceware.org/gdb/wiki/GDB_7.9_Release

(which means you need a PR number as well).
  
Eli Zaretskii June 4, 2015, 2:41 a.m. UTC | #5
> Date: Wed, 3 Jun 2015 15:01:41 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: qiyaoltc@gmail.com, gdb-patches@sourceware.org
> 
> Regardless, the procedure when pushing changes to the branch after
> a release has been made is to update the wiki:
> 
>     https://sourceware.org/gdb/wiki/GDB_7.9_Release
> 
> (which means you need a PR number as well).

That's a nuisance.
  
Joel Brobecker June 4, 2015, 4:08 a.m. UTC | #6
Eli,

> > Regardless, the procedure when pushing changes to the branch after
> > a release has been made is to update the wiki:
> > 
> >     https://sourceware.org/gdb/wiki/GDB_7.9_Release
> > 
> > (which means you need a PR number as well).
> 
> That's a nuisance.

Creating a PR: 5 mins. Updating the Wiki; just copy/paste the PR
number and the PR subject: 2 mins.

We have been down this road before. I have explained this to you
the last time and you've gone through the procedure already, so
you should know about it. You did not follow the procedure this
second time, and I just assumed that it was because you had forgotten.
But now that I sent you a reminder, you decided to be difficult about
it instead. I've been nice to you the last time, and did part of
the work for you. Is that really how you want to treat my request?

I did not make up that procedure. It was decided by us as a group.
If you have a better idea, by all means, propose it here, and we
can discuss.  But in the meantime, I expect you to follow what
has been decided by the group without making it extra work for me
every single time. Not to mention an unpleasant moment.
  
Eli Zaretskii June 4, 2015, 2:56 p.m. UTC | #7
> Date: Wed, 3 Jun 2015 21:08:20 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: qiyaoltc@gmail.com, gdb-patches@sourceware.org
> 
> Eli,
> 
> > > Regardless, the procedure when pushing changes to the branch after
> > > a release has been made is to update the wiki:
> > > 
> > >     https://sourceware.org/gdb/wiki/GDB_7.9_Release
> > > 
> > > (which means you need a PR number as well).
> > 
> > That's a nuisance.
> 
> Creating a PR: 5 mins. Updating the Wiki; just copy/paste the PR
> number and the PR subject: 2 mins.

Not for me: I don't even have a login in at least one of these places,
and the last time I tried to make a PR it took me an inordinate amount
of time.

> We have been down this road before. I have explained this to you
> the last time and you've gone through the procedure already, so
> you should know about it. You did not follow the procedure this
> second time, and I just assumed that it was because you had forgotten.

I did forget.  This requirement is so unique and different from any
other project in which I'm active that I simply forget, especially
that I don't push changes too frequently.

> But now that I sent you a reminder, you decided to be difficult about
> it instead. I've been nice to you the last time, and did part of
> the work for you. Is that really how you want to treat my request?
> 
> I did not make up that procedure. It was decided by us as a group.
> If you have a better idea, by all means, propose it here, and we
> can discuss.  But in the meantime, I expect you to follow what
> has been decided by the group without making it extra work for me
> every single time. Not to mention an unpleasant moment.

Sorry.  Maybe I should simply retire, to avoid annoying everyone.
  
Joel Brobecker June 9, 2015, 6:36 p.m. UTC | #8
> > Creating a PR: 5 mins. Updating the Wiki; just copy/paste the PR
> > number and the PR subject: 2 mins.
> 
> Not for me: I don't even have a login in at least one of these places,
> and the last time I tried to make a PR it took me an inordinate amount
> of time.

I've had a few days to think about this, and here is what I propose:
We both give; you spend the time to create a GDB wiki account and
send me your wiki login so I can add you to the list of authorized
contributors (allowing you to edit). In exchange, I create the PR
for you and update the wiki this time.
  
Eli Zaretskii June 9, 2015, 6:49 p.m. UTC | #9
> Date: Tue, 9 Jun 2015 14:36:13 -0400
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> > > Creating a PR: 5 mins. Updating the Wiki; just copy/paste the PR
> > > number and the PR subject: 2 mins.
> > 
> > Not for me: I don't even have a login in at least one of these places,
> > and the last time I tried to make a PR it took me an inordinate amount
> > of time.
> 
> I've had a few days to think about this, and here is what I propose:
> We both give; you spend the time to create a GDB wiki account and
> send me your wiki login so I can add you to the list of authorized
> contributors (allowing you to edit). In exchange, I create the PR
> for you and update the wiki this time.

Fine with me, but each time I asked how to create a login on the wiki,
you helpfully proposed to do that part for me "just this once".  So I
still don't know how to go about that.

By contrast, I do have a Bugzilla account.
  
Joel Brobecker June 9, 2015, 6:56 p.m. UTC | #10
> Fine with me, but each time I asked how to create a login on the wiki,
> you helpfully proposed to do that part for me "just this once".  So I
> still don't know how to go about that.

It's very similar to most sites I know. But here goes: Click on "you can
create one now" at the following URL:

https://sourceware.org/gdb/wiki/HomePage?action=login
  

Patch

diff --git a/gdb/go32-nat.c b/gdb/go32-nat.c
index f3966cd..4f5c2d2 100644
--- a/gdb/go32-nat.c
+++ b/gdb/go32-nat.c
@@ -587,10 +587,12 @@ 
   else
     res = read_child (memaddr, readbuf, len);
 
-  if (res <= 0)
+  /* read_child and write_child return zero on success, non-zero on
+     failure.  */
+  if (res != 0)
     return TARGET_XFER_E_IO;
 
-  *xfered_len = res;
+  *xfered_len = len;
   return TARGET_XFER_OK;
 }