* Feedback sought: patch to enable gdb to build on macOS Mojave
@ 2019-02-18 17:41 Joubert Nel
2019-02-18 20:38 ` Kevin Buettner
0 siblings, 1 reply; 7+ messages in thread
From: Joubert Nel @ 2019-02-18 17:41 UTC (permalink / raw)
To: gdb-patches
Hi,
I worked through some issues to get gdb to build on macOS Mojave
Specifically, I modified darwin-nat.c to overcome variable shadowing warnings (which get treated as errors by the build process). See point (b) at https://stackoverflow.com/questions/54736753/gdb-on-mac-mojave-during-startup-program-terminated-with-signal-unknown-sign
Worth submitting a patch so that others can easily build?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Feedback sought: patch to enable gdb to build on macOS Mojave
2019-02-18 17:41 Feedback sought: patch to enable gdb to build on macOS Mojave Joubert Nel
@ 2019-02-18 20:38 ` Kevin Buettner
2019-02-18 22:47 ` Tom Tromey
0 siblings, 1 reply; 7+ messages in thread
From: Kevin Buettner @ 2019-02-18 20:38 UTC (permalink / raw)
To: gdb-patches; +Cc: Joubert Nel
On Mon, 18 Feb 2019 09:41:34 -0800
Joubert Nel <joubert@joubster.com> wrote:
> I worked through some issues to get gdb to build on macOS Mojave
>
> Specifically, I modified darwin-nat.c to overcome variable shadowing warnings (which get treated as errors by the build process). See point (b) at https://stackoverflow.com/questions/54736753/gdb-on-mac-mojave-during-startup-program-terminated-with-signal-unknown-sign
>
> Worth submitting a patch so that others can easily build?
We'd definitely like to have your shadowing fixes for darwin-nat.c
(plus fixes for anything else you ran into as well).
Kevin
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Feedback sought: patch to enable gdb to build on macOS Mojave
2019-02-18 20:38 ` Kevin Buettner
@ 2019-02-18 22:47 ` Tom Tromey
2019-02-20 6:50 ` Joubert Nel
0 siblings, 1 reply; 7+ messages in thread
From: Tom Tromey @ 2019-02-18 22:47 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches, Joubert Nel
>>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:
>> Worth submitting a patch so that others can easily build?
Kevin> We'd definitely like to have your shadowing fixes for darwin-nat.c
Kevin> (plus fixes for anything else you ran into as well).
+1 to what Kevin said.
Contribution instructions are here: https://sourceware.org/gdb/contribute/
Tom
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Feedback sought: patch to enable gdb to build on macOS Mojave
2019-02-18 22:47 ` Tom Tromey
@ 2019-02-20 6:50 ` Joubert Nel
2019-02-20 16:49 ` John Baldwin
0 siblings, 1 reply; 7+ messages in thread
From: Joubert Nel @ 2019-02-20 6:50 UTC (permalink / raw)
To: Tom Tromey, Kevin Buettner; +Cc: gdb-patches
Great.
I read the contribution checklist and I think those are all great guidelines.
Before I prepare a formal submission email, it would be helpful to hear whether the actual code differences seem acceptable: https://gist.github.com/joubertnel/267ca0fff4eaad494cc19ec3ba7953ed
> On Feb 18, 2019, at 2:47 PM, Tom Tromey <tom@tromey.com> wrote:
>
>>>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:
>
>>> Worth submitting a patch so that others can easily build?
>
> Kevin> We'd definitely like to have your shadowing fixes for darwin-nat.c
> Kevin> (plus fixes for anything else you ran into as well).
>
> +1 to what Kevin said.
> Contribution instructions are here: https://sourceware.org/gdb/contribute/
>
> Tom
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Feedback sought: patch to enable gdb to build on macOS Mojave
2019-02-20 6:50 ` Joubert Nel
@ 2019-02-20 16:49 ` John Baldwin
2019-02-23 23:14 ` Joubert Nel
0 siblings, 1 reply; 7+ messages in thread
From: John Baldwin @ 2019-02-20 16:49 UTC (permalink / raw)
To: Joubert Nel, Tom Tromey, Kevin Buettner; +Cc: gdb-patches
On 2/19/19 10:50 PM, Joubert Nel wrote:
> Great.
>
> I read the contribution checklist and I think those are all great guidelines.
>
> Before I prepare a formal submission email, it would be helpful to hear whether the actual code differences seem acceptable: https://gist.github.com/joubertnel/267ca0fff4eaad494cc19ec3ba7953ed
Especially for a patch that small it is easier to review it inline.
I think the first hunk is fine.
The second one probably isn't the right fix. The earlier 'res' is an
'int' for the return value from darwin_decode_notify_message. The
later one is a pid_t for the return value from wait4. I would perhaps
rename the second 'res' to 'wpid' instead of using the int.
For the third one it looks like you can just remove the line entirely.
priv is already set to 'get_darwin_inferior (inf)' at the start of the
function, so that assignment is redundant.
--
John Baldwin
                                                                           Â
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Feedback sought: patch to enable gdb to build on macOS Mojave
2019-02-20 16:49 ` John Baldwin
@ 2019-02-23 23:14 ` Joubert Nel
2019-02-25 22:41 ` John Baldwin
0 siblings, 1 reply; 7+ messages in thread
From: Joubert Nel @ 2019-02-23 23:14 UTC (permalink / raw)
To: John Baldwin, simon.marchi, Tom Tromey, Kevin Buettner, gdb-patches
Thanks for the review, John & Simon.
Your feedback makes sense and I’ve revised the suggested patch accordingly: https://gist.github.com/joubertnel/267ca0fff4eaad494cc19ec3ba7953ed <https://gist.github.com/joubertnel/267ca0fff4eaad494cc19ec3ba7953ed>
> On Feb 20, 2019, at 8:48 AM, John Baldwin <jhb@FreeBSD.org> wrote:
>
> On 2/19/19 10:50 PM, Joubert Nel wrote:
>> Great.
>>
>> I read the contribution checklist and I think those are all great guidelines.
>>
>> Before I prepare a formal submission email, it would be helpful to hear whether the actual code differences seem acceptable: https://gist.github.com/joubertnel/267ca0fff4eaad494cc19ec3ba7953ed
>
> Especially for a patch that small it is easier to review it inline.
>
> I think the first hunk is fine.
>
> The second one probably isn't the right fix. The earlier 'res' is an
> 'int' for the return value from darwin_decode_notify_message. The
> later one is a pid_t for the return value from wait4. I would perhaps
> rename the second 'res' to 'wpid' instead of using the int.
>
> For the third one it looks like you can just remove the line entirely.
> priv is already set to 'get_darwin_inferior (inf)' at the start of the
> function, so that assignment is redundant.
>
> --
> John Baldwin
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Feedback sought: patch to enable gdb to build on macOS Mojave
2019-02-23 23:14 ` Joubert Nel
@ 2019-02-25 22:41 ` John Baldwin
0 siblings, 0 replies; 7+ messages in thread
From: John Baldwin @ 2019-02-25 22:41 UTC (permalink / raw)
To: Joubert Nel, simon.marchi, Tom Tromey, Kevin Buettner, gdb-patches
On 2/23/19 3:14 PM, Joubert Nel wrote:
> Thanks for the review, John & Simon.
>
> Your feedback makes sense and Iâve revised the suggested patch accordingly: https://gist.github.com/joubertnel/267ca0fff4eaad494cc19ec3ba7953ed <https://gist.github.com/joubertnel/267ca0fff4eaad494cc19ec3ba7953ed>
I think this looks fine, but you should send it as a normal patch as Kevin
and Tom noted previously.
--
John Baldwin
                                                                           Â
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-02-25 22:41 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-18 17:41 Feedback sought: patch to enable gdb to build on macOS Mojave Joubert Nel
2019-02-18 20:38 ` Kevin Buettner
2019-02-18 22:47 ` Tom Tromey
2019-02-20 6:50 ` Joubert Nel
2019-02-20 16:49 ` John Baldwin
2019-02-23 23:14 ` Joubert Nel
2019-02-25 22:41 ` John Baldwin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).