public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Fix calling gcore when gdb is not in $PATH.
@ 2013-10-11 14:10 Luis Machado
  2013-10-11 14:31 ` Jan Kratochvil
  0 siblings, 1 reply; 12+ messages in thread
From: Luis Machado @ 2013-10-11 14:10 UTC (permalink / raw)
  To: 'gdb-patches@sourceware.org', Jan Kratochvil

[-- Attachment #1: Type: text/plain, Size: 868 bytes --]

Hi,

Currently, the generated gcore scripts assume the gdb binary they need 
to call is present in $PATH, but this may not always be true.

If you want to call the gcore scripts from a relative directory, for 
example, it will fail to locate the gdb binary. The attached patch fixes 
this.

Before the patch:

$ /tmp/gdb_test_install/bin/gcoreblahbleh 20000
/tmp/gdb_test_install/bin/gcoreblahbleh: 54: 
/tmp/gdb_test_install/bin/gcoreblahbleh: gdbblahbleh: not found
gcoreblahbleh: failed to create core.20000

After the patch:

$ /tmp/gdb_test_install/bin/gcoreblahbleh 20000
warning: unable to open /proc file '/proc/20000/status'
warning: unable to open /proc file '/proc/20000/status'
ptrace: No such process.
You can't do that without a process to debug.
The program is not being run.
gcoreblahbleh: failed to create core.20000

How does it look?

Regards,
Luis

[-- Attachment #2: gcore.diff --]
[-- Type: text/x-patch, Size: 654 bytes --]

2013-10-11  Luis Machado  <lgustavo@codesourcery.com>

	* gcore.in: Call gdb using the full path to the gcore script.

diff --git a/gdb/gcore.in b/gdb/gcore.in
index 9c5b14d..b82479c 100644
--- a/gdb/gcore.in
+++ b/gdb/gcore.in
@@ -51,7 +51,7 @@ for pid in $*
 do
 	# `</dev/null' to avoid touching interactive terminal if it is
 	# available but not accessible as GDB would get stopped on SIGTTIN.
-	@GDB_TRANSFORM_NAME@ </dev/null --nx --batch \
+	"$(dirname "$0")"/@GDB_TRANSFORM_NAME@ </dev/null --nx --batch \
 	    -ex "set pagination off" -ex "set height 0" -ex "set width 0" \
 	    -ex "attach $pid" -ex "gcore $name.$pid" -ex detach -ex quit
 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Fix calling gcore when gdb is not in $PATH.
  2013-10-11 14:10 [PATCH] Fix calling gcore when gdb is not in $PATH Luis Machado
@ 2013-10-11 14:31 ` Jan Kratochvil
  2013-10-11 14:46   ` Luis Machado
  0 siblings, 1 reply; 12+ messages in thread
From: Jan Kratochvil @ 2013-10-11 14:31 UTC (permalink / raw)
  To: Luis Machado; +Cc: 'gdb-patches@sourceware.org'

On Fri, 11 Oct 2013 16:10:16 +0200, Luis Machado wrote:
> --- a/gdb/gcore.in
> +++ b/gdb/gcore.in
> @@ -51,7 +51,7 @@ for pid in $*
>  do
>  	# `</dev/null' to avoid touching interactive terminal if it is
>  	# available but not accessible as GDB would get stopped on SIGTTIN.
> -	@GDB_TRANSFORM_NAME@ </dev/null --nx --batch \
> +	"$(dirname "$0")"/@GDB_TRANSFORM_NAME@ </dev/null --nx --batch \

I have only some concern if $0 does not contain a directory name.
Then `dirname basename` will be . and gdb -> ./gdb will be a regression as
./gdb will typically not be found.

For example if you run:
	$ sh gcore foo
then sh (or bash) executes /usr/bin/gcore but $0 is still just "gcore".

It IMO even corresponds to the sh $0 POSIX description ("command_file"):
	http://pubs.opengroup.org/onlinepubs/007908799/xcu/sh.html


Thanks,
Jan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Fix calling gcore when gdb is not in $PATH.
  2013-10-11 14:31 ` Jan Kratochvil
@ 2013-10-11 14:46   ` Luis Machado
  2013-10-11 16:46     ` Luis Machado
  0 siblings, 1 reply; 12+ messages in thread
From: Luis Machado @ 2013-10-11 14:46 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: 'gdb-patches@sourceware.org'

On 10/11/2013 11:31 AM, Jan Kratochvil wrote:
> On Fri, 11 Oct 2013 16:10:16 +0200, Luis Machado wrote:
>> --- a/gdb/gcore.in
>> +++ b/gdb/gcore.in
>> @@ -51,7 +51,7 @@ for pid in $*
>>   do
>>   	# `</dev/null' to avoid touching interactive terminal if it is
>>   	# available but not accessible as GDB would get stopped on SIGTTIN.
>> -	@GDB_TRANSFORM_NAME@ </dev/null --nx --batch \
>> +	"$(dirname "$0")"/@GDB_TRANSFORM_NAME@ </dev/null --nx --batch \
>
> I have only some concern if $0 does not contain a directory name.
> Then `dirname basename` will be . and gdb -> ./gdb will be a regression as
> ./gdb will typically not be found.
>
> For example if you run:
> 	$ sh gcore foo
> then sh (or bash) executes /usr/bin/gcore but $0 is still just "gcore".
>
> It IMO even corresponds to the sh $0 POSIX description ("command_file"):
> 	http://pubs.opengroup.org/onlinepubs/007908799/xcu/sh.html


Right. That situation can indeed happen. Though it looks a little 
awkward to call something in your path like that.

I suppose we can extend the check to cover that case as well. Let me go 
back to the drawing board.

Thanks,
Luis

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Fix calling gcore when gdb is not in $PATH.
  2013-10-11 14:46   ` Luis Machado
@ 2013-10-11 16:46     ` Luis Machado
  2013-10-11 16:56       ` Jan Kratochvil
  0 siblings, 1 reply; 12+ messages in thread
From: Luis Machado @ 2013-10-11 16:46 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: 'gdb-patches@sourceware.org'

[-- Attachment #1: Type: text/plain, Size: 1297 bytes --]

On 10/11/2013 11:46 AM, Luis Machado wrote:
> On 10/11/2013 11:31 AM, Jan Kratochvil wrote:
>> On Fri, 11 Oct 2013 16:10:16 +0200, Luis Machado wrote:
>>> --- a/gdb/gcore.in
>>> +++ b/gdb/gcore.in
>>> @@ -51,7 +51,7 @@ for pid in $*
>>>   do
>>>       # `</dev/null' to avoid touching interactive terminal if it is
>>>       # available but not accessible as GDB would get stopped on
>>> SIGTTIN.
>>> -    @GDB_TRANSFORM_NAME@ </dev/null --nx --batch \
>>> +    "$(dirname "$0")"/@GDB_TRANSFORM_NAME@ </dev/null --nx --batch \
>>
>> I have only some concern if $0 does not contain a directory name.
>> Then `dirname basename` will be . and gdb -> ./gdb will be a
>> regression as
>> ./gdb will typically not be found.
>>
>> For example if you run:
>>     $ sh gcore foo
>> then sh (or bash) executes /usr/bin/gcore but $0 is still just "gcore".
>>
>> It IMO even corresponds to the sh $0 POSIX description ("command_file"):
>>     http://pubs.opengroup.org/onlinepubs/007908799/xcu/sh.html
>
>
> Right. That situation can indeed happen. Though it looks a little
> awkward to call something in your path like that.
>
> I suppose we can extend the check to cover that case as well. Let me go
> back to the drawing board.

Is this one more acceptable (though slightly less portable)?

Regards,
Luis


[-- Attachment #2: gcore.diff --]
[-- Type: text/x-patch, Size: 1271 bytes --]

2013-10-11  Luis Machado  <lgustavo@codesourcery.com>

	* gcore.in: Call gdb using the full path to the gcore script.

diff --git a/gdb/gcore.in b/gdb/gcore.in
index 9c5b14d..dc6a8f9 100644
--- a/gdb/gcore.in
+++ b/gdb/gcore.in
@@ -49,9 +49,26 @@ rc=0
 # Loop through pids
 for pid in $*
 do
+# Attempt to fetch the absolute path to the gcore script that was
+# called.
+binary_path=`dirname "$0"`
+
+	if test "x$binary_path" = x. ; then
+	  # We got "." back as a path.  This means the user executed
+	  # the gcore script locally (i.e. ./gcore) or called the
+	  # script via a shell interpreter (i.e. sh gcore).  We use
+	  # the "which" command to locate the real path of the gcore
+	  # script, disambiguating this situation.
+	  binary_path_from_env=`which "$0"`
+	  binary_path=`dirname $binary_path_from_env`
+	fi
+
+	# Add a slash to the path.
+	binary_path="$binary_path/"
+
 	# `</dev/null' to avoid touching interactive terminal if it is
 	# available but not accessible as GDB would get stopped on SIGTTIN.
-	@GDB_TRANSFORM_NAME@ </dev/null --nx --batch \
+	"$binary_path"@GDB_TRANSFORM_NAME@ </dev/null --nx --batch \
 	    -ex "set pagination off" -ex "set height 0" -ex "set width 0" \
 	    -ex "attach $pid" -ex "gcore $name.$pid" -ex detach -ex quit
 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Fix calling gcore when gdb is not in $PATH.
  2013-10-11 16:46     ` Luis Machado
@ 2013-10-11 16:56       ` Jan Kratochvil
  2013-10-11 17:53         ` Luis Machado
  0 siblings, 1 reply; 12+ messages in thread
From: Jan Kratochvil @ 2013-10-11 16:56 UTC (permalink / raw)
  To: Luis Machado; +Cc: 'gdb-patches@sourceware.org'

On Fri, 11 Oct 2013 18:46:15 +0200, Luis Machado wrote:
> --- a/gdb/gcore.in
> +++ b/gdb/gcore.in
> @@ -49,9 +49,26 @@ rc=0
>  # Loop through pids
>  for pid in $*
>  do
> +# Attempt to fetch the absolute path to the gcore script that was
> +# called.
> +binary_path=`dirname "$0"`
> +
> +	if test "x$binary_path" = x. ; then
> +	  # We got "." back as a path.  This means the user executed
> +	  # the gcore script locally (i.e. ./gcore) or called the
> +	  # script via a shell interpreter (i.e. sh gcore).  We use
> +	  # the "which" command to locate the real path of the gcore
> +	  # script, disambiguating this situation.
> +	  binary_path_from_env=`which "$0"`
> +	  binary_path=`dirname $binary_path_from_env`

In generally OK, just still ... is there some reason for this 'which' search?
Moreover if one really runs ./gcore then it should IMO take ./gdb (and not some
other gdb), if we should really pick GDB from the directory of gcore.

What about:
	if test "`basename "$0"`" != "$0"; then
	  binary_path="`dirname "$0"`/"
	else
	  binary_path=""
	fi

The first condition I write as:
	if test "${0#*/}" != "$0";then
but I have no idea if MinGW with its \ in paths can run a shell script.



> +	fi
> +
> +	# Add a slash to the path.
> +	binary_path="$binary_path/"
> +
>  	# `</dev/null' to avoid touching interactive terminal if it is
>  	# available but not accessible as GDB would get stopped on SIGTTIN.
> -	@GDB_TRANSFORM_NAME@ </dev/null --nx --batch \
> +	"$binary_path"@GDB_TRANSFORM_NAME@ </dev/null --nx --batch \
>  	    -ex "set pagination off" -ex "set height 0" -ex "set width 0" \
>  	    -ex "attach $pid" -ex "gcore $name.$pid" -ex detach -ex quit
>  


Thanks,
Jan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Fix calling gcore when gdb is not in $PATH.
  2013-10-11 16:56       ` Jan Kratochvil
@ 2013-10-11 17:53         ` Luis Machado
  2013-10-11 18:10           ` Jan Kratochvil
  0 siblings, 1 reply; 12+ messages in thread
From: Luis Machado @ 2013-10-11 17:53 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: 'gdb-patches@sourceware.org'

On 10/11/2013 01:56 PM, Jan Kratochvil wrote:
> On Fri, 11 Oct 2013 18:46:15 +0200, Luis Machado wrote:
>> --- a/gdb/gcore.in
>> +++ b/gdb/gcore.in
>> @@ -49,9 +49,26 @@ rc=0
>>   # Loop through pids
>>   for pid in $*
>>   do
>> +# Attempt to fetch the absolute path to the gcore script that was
>> +# called.
>> +binary_path=`dirname "$0"`
>> +
>> +	if test "x$binary_path" = x. ; then
>> +	  # We got "." back as a path.  This means the user executed
>> +	  # the gcore script locally (i.e. ./gcore) or called the
>> +	  # script via a shell interpreter (i.e. sh gcore).  We use
>> +	  # the "which" command to locate the real path of the gcore
>> +	  # script, disambiguating this situation.
>> +	  binary_path_from_env=`which "$0"`
>> +	  binary_path=`dirname $binary_path_from_env`
>
> In generally OK, just still ... is there some reason for this 'which' search?
> Moreover if one really runs ./gcore then it should IMO take ./gdb (and not some
> other gdb), if we should really pick GDB from the directory of gcore.

Yes. The reason is to pick the gdb binary from the directory that 
contains the gcore script the user invoked.

If the user issued "sh gcore" and /usr/bin/gcore was picked (based on 
$PATH), then we should use /usr/bin/gdb.

Now, if the user issued "./gcore", ./gdb will be picked up, and so on.

Does it make sense?

Luis

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Fix calling gcore when gdb is not in $PATH.
  2013-10-11 17:53         ` Luis Machado
@ 2013-10-11 18:10           ` Jan Kratochvil
  2013-10-11 18:22             ` Luis Machado
  0 siblings, 1 reply; 12+ messages in thread
From: Jan Kratochvil @ 2013-10-11 18:10 UTC (permalink / raw)
  To: Luis Machado; +Cc: 'gdb-patches@sourceware.org'

On Fri, 11 Oct 2013 19:53:12 +0200, Luis Machado wrote:
> On 10/11/2013 01:56 PM, Jan Kratochvil wrote:
> >On Fri, 11 Oct 2013 18:46:15 +0200, Luis Machado wrote:
> >>--- a/gdb/gcore.in
> >>+++ b/gdb/gcore.in
> >>@@ -49,9 +49,26 @@ rc=0
> >>  # Loop through pids
> >>  for pid in $*
> >>  do
> >>+# Attempt to fetch the absolute path to the gcore script that was
> >>+# called.
> >>+binary_path=`dirname "$0"`
> >>+
> >>+	if test "x$binary_path" = x. ; then
> >>+	  # We got "." back as a path.  This means the user executed
> >>+	  # the gcore script locally (i.e. ./gcore) or called the
> >>+	  # script via a shell interpreter (i.e. sh gcore).  We use
> >>+	  # the "which" command to locate the real path of the gcore
> >>+	  # script, disambiguating this situation.
> >>+	  binary_path_from_env=`which "$0"`
> >>+	  binary_path=`dirname $binary_path_from_env`
> >
> >In generally OK, just still ... is there some reason for this 'which' search?
> >Moreover if one really runs ./gcore then it should IMO take ./gdb (and not some
> >other gdb), if we should really pick GDB from the directory of gcore.
> 
> Yes. The reason is to pick the gdb binary from the directory that
> contains the gcore script the user invoked.
> 
> If the user issued "sh gcore" and /usr/bin/gcore was picked (based
> on $PATH), then we should use /usr/bin/gdb.
> 
> Now, if the user issued "./gcore", ./gdb will be picked up, and so on.
> 
> Does it make sense?

Great we agree.  But your code does pick /usr/bin/gdb for ./gcore, doesn't it?
Which is why I proposed the change I proposed.


Jan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Fix calling gcore when gdb is not in $PATH.
  2013-10-11 18:10           ` Jan Kratochvil
@ 2013-10-11 18:22             ` Luis Machado
  2013-10-11 19:58               ` Jan Kratochvil
  0 siblings, 1 reply; 12+ messages in thread
From: Luis Machado @ 2013-10-11 18:22 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: 'gdb-patches@sourceware.org'

On 10/11/2013 03:10 PM, Jan Kratochvil wrote:
> On Fri, 11 Oct 2013 19:53:12 +0200, Luis Machado wrote:
>> On 10/11/2013 01:56 PM, Jan Kratochvil wrote:
>>> On Fri, 11 Oct 2013 18:46:15 +0200, Luis Machado wrote:
>>>> --- a/gdb/gcore.in
>>>> +++ b/gdb/gcore.in
>>>> @@ -49,9 +49,26 @@ rc=0
>>>>   # Loop through pids
>>>>   for pid in $*
>>>>   do
>>>> +# Attempt to fetch the absolute path to the gcore script that was
>>>> +# called.
>>>> +binary_path=`dirname "$0"`
>>>> +
>>>> +	if test "x$binary_path" = x. ; then
>>>> +	  # We got "." back as a path.  This means the user executed
>>>> +	  # the gcore script locally (i.e. ./gcore) or called the
>>>> +	  # script via a shell interpreter (i.e. sh gcore).  We use
>>>> +	  # the "which" command to locate the real path of the gcore
>>>> +	  # script, disambiguating this situation.
>>>> +	  binary_path_from_env=`which "$0"`
>>>> +	  binary_path=`dirname $binary_path_from_env`
>>>
>>> In generally OK, just still ... is there some reason for this 'which' search?
>>> Moreover if one really runs ./gcore then it should IMO take ./gdb (and not some
>>> other gdb), if we should really pick GDB from the directory of gcore.
>>
>> Yes. The reason is to pick the gdb binary from the directory that
>> contains the gcore script the user invoked.
>>
>> If the user issued "sh gcore" and /usr/bin/gcore was picked (based
>> on $PATH), then we should use /usr/bin/gdb.
>>
>> Now, if the user issued "./gcore", ./gdb will be picked up, and so on.
>>
>> Does it make sense?
>
> Great we agree.  But your code does pick /usr/bin/gdb for ./gcore, doesn't it?
> Which is why I proposed the change I proposed.

Hmmm... unless there is some discrepancy between shell interpreters, 
mine (bash) does the following:

Invocation: ./gcore
dirname ./gcore -> .

So we go inside the if block and locate the real binary that is being 
called based on $PATH.

which ./gcore -> ./gcore

So we pick the dirname of ./gcore (which is ".") and add a slash to it, 
resulting in ./gdb.

Do you see something different or a corner case?

Luis

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Fix calling gcore when gdb is not in $PATH.
  2013-10-11 18:22             ` Luis Machado
@ 2013-10-11 19:58               ` Jan Kratochvil
  2013-10-14 12:24                 ` Luis Machado
  0 siblings, 1 reply; 12+ messages in thread
From: Jan Kratochvil @ 2013-10-11 19:58 UTC (permalink / raw)
  To: Luis Machado; +Cc: 'gdb-patches@sourceware.org'

On Fri, 11 Oct 2013 20:22:05 +0200, Luis Machado wrote:
> Hmmm... unless there is some discrepancy between shell interpreters,
> mine (bash) does the following:

OK, true, it works thanks to the 'which' command there.

But then why you have there the conditional
	if test "x$binary_path" = x. ; then
?
You can run the 'which' block every time and it will work.


Jan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Fix calling gcore when gdb is not in $PATH.
  2013-10-11 19:58               ` Jan Kratochvil
@ 2013-10-14 12:24                 ` Luis Machado
  2013-10-15 15:10                   ` Jan Kratochvil
  0 siblings, 1 reply; 12+ messages in thread
From: Luis Machado @ 2013-10-14 12:24 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: 'gdb-patches@sourceware.org'

[-- Attachment #1: Type: text/plain, Size: 903 bytes --]

On 10/11/2013 04:00 PM, Jan Kratochvil wrote:
> On Fri, 11 Oct 2013 20:22:05 +0200, Luis Machado wrote:
>> Hmmm... unless there is some discrepancy between shell interpreters,
>> mine (bash) does the following:
>
> OK, true, it works thanks to the 'which' command there.
>
> But then why you have there the conditional
> 	if test "x$binary_path" = x. ; then
> ?
> You can run the 'which' block every time and it will work.

That's true. Though i've noticed that the following gives an unexpected 
result...

Invocation: sh gcore (with gcore living in ".")
The /usr/bin/gdb binary gets picked up, because "which gcore" returns 
nothing even if gcore lives in ".". *sigh*.

An additional check needs to be done. The attached update patch 
accomplishes this. It seems to cover all the scenarios.

I decided to add a chunk of code to error out in case the correct GDB 
binary is not found.

Thoughts?

Luis

[-- Attachment #2: gcore.diff --]
[-- Type: text/x-patch, Size: 2042 bytes --]

2013-10-14  Luis Machado  <lgustavo@codesourcery.com>

	* gcore.in: Call GDB using the full path to the gcore script.
	Error out if the GDB binary is not found.

diff --git a/gdb/gcore.in b/gdb/gcore.in
index 9c5b14d..cbc8f06 100644
--- a/gdb/gcore.in
+++ b/gdb/gcore.in
@@ -43,6 +43,40 @@ then
     shift; shift
 fi
 
+# Attempt to fetch the absolute path to the gcore script that was
+# called.
+binary_path=`dirname "$0"`
+
+if test "x$binary_path" = x. ; then
+  # We got "." back as a path.  This means the user executed
+  # the gcore script locally (i.e. ./gcore) or called the
+  # script via a shell interpreter (i.e. sh gcore).
+  binary_basename=`basename "$0"`
+
+  # If the gcore script was called like "sh gcore" and the script
+  # lives in the current directory, "which" will not give us "gcore".
+  # So first we check if the script is in the current directory
+  # before using the output "which".
+  if test -f "$binary_basename" ; then
+    # We have a local gcore script in ".".  This covers the case of
+    # doing "./gcore" or "sh gcore".
+    binary_path="."
+  else
+    # The gcore script was not found in ".", which means the script
+    # was called from somewhere else in $PATH.  Extract the correct
+    # path now.
+    binary_path_from_env=`which "$0"`
+    binary_path=`dirname "$binary_path_from_env"`
+  fi
+fi
+
+# Check if the GDB binary is in the expected path.  If not, just
+# quit with a message.
+if [ ! -f "$binary_path"/@GDB_TRANSFORM_NAME@ ]; then
+  echo "gcore: GDB binary (${binary_path}/@GDB_TRANSFORM_NAME@) not found"
+  exit 1
+fi
+
 # Initialise return code.
 rc=0
 
@@ -51,7 +85,7 @@ for pid in $*
 do
 	# `</dev/null' to avoid touching interactive terminal if it is
 	# available but not accessible as GDB would get stopped on SIGTTIN.
-	@GDB_TRANSFORM_NAME@ </dev/null --nx --batch \
+	$binary_path/@GDB_TRANSFORM_NAME@ </dev/null --nx --batch \
 	    -ex "set pagination off" -ex "set height 0" -ex "set width 0" \
 	    -ex "attach $pid" -ex "gcore $name.$pid" -ex detach -ex quit
 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Fix calling gcore when gdb is not in $PATH.
  2013-10-14 12:24                 ` Luis Machado
@ 2013-10-15 15:10                   ` Jan Kratochvil
  2013-10-16 15:13                     ` Luis Machado
  0 siblings, 1 reply; 12+ messages in thread
From: Jan Kratochvil @ 2013-10-15 15:10 UTC (permalink / raw)
  To: Luis Machado; +Cc: 'gdb-patches@sourceware.org'

On Mon, 14 Oct 2013 14:24:09 +0200, Luis Machado wrote:
> --- a/gdb/gcore.in
> +++ b/gdb/gcore.in
> @@ -43,6 +43,40 @@ then
>      shift; shift
>  fi
>  
> +# Attempt to fetch the absolute path to the gcore script that was
> +# called.
> +binary_path=`dirname "$0"`
> +
> +if test "x$binary_path" = x. ; then

I find this test needlessly complicating the code a bit as it is true for both
"sh gcore" and for "./gcore" while the conditionalized block of code is needed
only in the "sh gcore" case.  I was proposing a different test before.  But it
works even for the "./gcore" execution case so technically it is correct.


> +  # We got "." back as a path.  This means the user executed
> +  # the gcore script locally (i.e. ./gcore) or called the
> +  # script via a shell interpreter (i.e. sh gcore).
> +  binary_basename=`basename "$0"`
> +
> +  # If the gcore script was called like "sh gcore" and the script
> +  # lives in the current directory, "which" will not give us "gcore".
> +  # So first we check if the script is in the current directory
> +  # before using the output "which".
> +  if test -f "$binary_basename" ; then
> +    # We have a local gcore script in ".".  This covers the case of
> +    # doing "./gcore" or "sh gcore".
> +    binary_path="."
> +  else
> +    # The gcore script was not found in ".", which means the script
> +    # was called from somewhere else in $PATH.  Extract the correct

       # was called from somewhere else in $PATH by "sh gcore". [...]


> +    # path now.
> +    binary_path_from_env=`which "$0"`
> +    binary_path=`dirname "$binary_path_from_env"`
> +  fi
> +fi
> +
> +# Check if the GDB binary is in the expected path.  If not, just
> +# quit with a message.
> +if [ ! -f "$binary_path"/@GDB_TRANSFORM_NAME@ ]; then
> +  echo "gcore: GDB binary (${binary_path}/@GDB_TRANSFORM_NAME@) not found"
> +  exit 1
> +fi
> +
>  # Initialise return code.
>  rc=0
>  
> @@ -51,7 +85,7 @@ for pid in $*
>  do
>  	# `</dev/null' to avoid touching interactive terminal if it is
>  	# available but not accessible as GDB would get stopped on SIGTTIN.
> -	@GDB_TRANSFORM_NAME@ </dev/null --nx --batch \
> +	$binary_path/@GDB_TRANSFORM_NAME@ </dev/null --nx --batch \
>  	    -ex "set pagination off" -ex "set height 0" -ex "set width 0" \
>  	    -ex "attach $pid" -ex "gcore $name.$pid" -ex detach -ex quit
>  

OK for check-in.


Thanks,
Jan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Fix calling gcore when gdb is not in $PATH.
  2013-10-15 15:10                   ` Jan Kratochvil
@ 2013-10-16 15:13                     ` Luis Machado
  0 siblings, 0 replies; 12+ messages in thread
From: Luis Machado @ 2013-10-16 15:13 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: 'gdb-patches@sourceware.org'

On 10/15/2013 12:10 PM, Jan Kratochvil wrote:
> On Mon, 14 Oct 2013 14:24:09 +0200, Luis Machado wrote:
>> --- a/gdb/gcore.in
>> +++ b/gdb/gcore.in
>> @@ -43,6 +43,40 @@ then
>>       shift; shift
>>   fi
>>
>> +# Attempt to fetch the absolute path to the gcore script that was
>> +# called.
>> +binary_path=`dirname "$0"`
>> +
>> +if test "x$binary_path" = x. ; then
>
> I find this test needlessly complicating the code a bit as it is true for both
> "sh gcore" and for "./gcore" while the conditionalized block of code is needed
> only in the "sh gcore" case.  I was proposing a different test before.  But it
> works even for the "./gcore" execution case so technically it is correct.
>
>

It is a bit messy due to the corner cases unfortunately.

>> +  # We got "." back as a path.  This means the user executed
>> +  # the gcore script locally (i.e. ./gcore) or called the
>> +  # script via a shell interpreter (i.e. sh gcore).
>> +  binary_basename=`basename "$0"`
>> +
>> +  # If the gcore script was called like "sh gcore" and the script
>> +  # lives in the current directory, "which" will not give us "gcore".
>> +  # So first we check if the script is in the current directory
>> +  # before using the output "which".
>> +  if test -f "$binary_basename" ; then
>> +    # We have a local gcore script in ".".  This covers the case of
>> +    # doing "./gcore" or "sh gcore".
>> +    binary_path="."
>> +  else
>> +    # The gcore script was not found in ".", which means the script
>> +    # was called from somewhere else in $PATH.  Extract the correct
>
>         # was called from somewhere else in $PATH by "sh gcore". [...]
>
>
>> +    # path now.
>> +    binary_path_from_env=`which "$0"`
>> +    binary_path=`dirname "$binary_path_from_env"`
>> +  fi
>> +fi
>> +
>> +# Check if the GDB binary is in the expected path.  If not, just
>> +# quit with a message.
>> +if [ ! -f "$binary_path"/@GDB_TRANSFORM_NAME@ ]; then
>> +  echo "gcore: GDB binary (${binary_path}/@GDB_TRANSFORM_NAME@) not found"
>> +  exit 1
>> +fi
>> +
>>   # Initialise return code.
>>   rc=0
>>
>> @@ -51,7 +85,7 @@ for pid in $*
>>   do
>>   	# `</dev/null' to avoid touching interactive terminal if it is
>>   	# available but not accessible as GDB would get stopped on SIGTTIN.
>> -	@GDB_TRANSFORM_NAME@ </dev/null --nx --batch \
>> +	$binary_path/@GDB_TRANSFORM_NAME@ </dev/null --nx --batch \
>>   	    -ex "set pagination off" -ex "set height 0" -ex "set width 0" \
>>   	    -ex "attach $pid" -ex "gcore $name.$pid" -ex detach -ex quit
>>
>
> OK for check-in.

Fixed and checked in. Thanks!

Luis

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2013-10-16 15:13 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-11 14:10 [PATCH] Fix calling gcore when gdb is not in $PATH Luis Machado
2013-10-11 14:31 ` Jan Kratochvil
2013-10-11 14:46   ` Luis Machado
2013-10-11 16:46     ` Luis Machado
2013-10-11 16:56       ` Jan Kratochvil
2013-10-11 17:53         ` Luis Machado
2013-10-11 18:10           ` Jan Kratochvil
2013-10-11 18:22             ` Luis Machado
2013-10-11 19:58               ` Jan Kratochvil
2013-10-14 12:24                 ` Luis Machado
2013-10-15 15:10                   ` Jan Kratochvil
2013-10-16 15:13                     ` Luis Machado

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).