Move 'is_regular_file' from common-utils.c to filestuff.c
Commit Message
There is no reason for 'is_regular_file' to be in common-utils.c; it
belongs to 'filestuff.c'. This commit moves the function definition
and its prototype to the appropriate files.
The motivation behind this move is a failure that happens on certain
cross-compilation environments when compiling the IPA library, due to
the way gnulib probes the need for a 'stat' call replacement. Because
configure checks when cross-compiling are more limited, gnulib decides
that it needs to substitute the 'stat' calls its own 'rpl_stat';
however, the IPA library doesn't link with gnulib, which leads to an
error when compiling 'common-utils.c':
...
/opt/x86-core2--musl--bleeding-edge-2018.09-1/bin/i686-buildroot-linux-musl-g++ -shared -fPIC -Wl,--soname=libinproctrace.so -Wl,--no-undefined -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os -I. -I. -I./../common -I./../regformats -I./.. -I./../../include -I./../gnulib/import -Ibuild-gnulib-gdbserver/import -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized -DGDBSERVER \
-Wl,--dynamic-list=./proc-service.list -o libinproctrace.so ax-ipa.o common-utils-ipa.o errors-ipa.o format-ipa.o print-utils-ipa.o regcache-ipa.o remote-utils-ipa.o rsp-low-ipa.o tdesc-ipa.o tracepoint-ipa.o utils-ipa.o vec-ipa.o linux-i386-ipa.o linux-x86-tdesc-ipa.o arch/i386-ipa.o -ldl -pthread
/opt/x86-core2--musl--bleeding-edge-2018.09-1/lib/gcc/i686-buildroot-linux-musl/8.2.0/../../../../i686-buildroot-linux-musl/bin/ld: common-utils-ipa.o: in function `is_regular_file(char const*, int*)':
common-utils.c:(.text+0x695): undefined reference to `rpl_stat'
collect2: error: ld returned 1 exit status
Makefile:413: recipe for target 'libinproctrace.so' failed
make[1]: *** [libinproctrace.so] Error 1
...
More details can also be found at:
https://sourceware.org/ml/gdb-patches/2018-09/msg00304.html
The most simple fix for this problem is to move 'is_regular_file' to
'filestuff.c', which is not used by IPA. This ends up making the
files more logically organized as well, since 'is_regular_file' is a
file operation.
No regressions found.
gdb/ChangeLog:
2018-09-12 Sergio Durigan Junior <sergiodj@redhat.com>
* common/common-utils.c: Don't include '<sys/stat.h>'.
(is_regular_file): Move to...
* common/filestuff.c (is_regular_file): ... here.
* common/common-utils.h (is_regular_file): Move to...
* common/filestuff.h (is_regular_file): ... here.
---
gdb/common/common-utils.c | 32 --------------------------------
gdb/common/common-utils.h | 5 -----
gdb/common/filestuff.c | 31 +++++++++++++++++++++++++++++++
gdb/common/filestuff.h | 5 +++++
4 files changed, 36 insertions(+), 37 deletions(-)
Comments
On 09/12/2018 06:31 PM, Sergio Durigan Junior wrote:
> The most simple fix for this problem is to move 'is_regular_file' to
> 'filestuff.c', which is not used by IPA. This ends up making the
> files more logically organized as well, since 'is_regular_file' is a
> file operation.
>
> No regressions found.
>
> gdb/ChangeLog:
> 2018-09-12 Sergio Durigan Junior <sergiodj@redhat.com>
>
> * common/common-utils.c: Don't include '<sys/stat.h>'.
> (is_regular_file): Move to...
> * common/filestuff.c (is_regular_file): ... here.
> * common/common-utils.h (is_regular_file): Move to...
> * common/filestuff.h (is_regular_file): ... here.
OK.
Thanks,
Pedro Alves
On Wednesday, September 12 2018, Pedro Alves wrote:
> On 09/12/2018 06:31 PM, Sergio Durigan Junior wrote:
>
>> The most simple fix for this problem is to move 'is_regular_file' to
>> 'filestuff.c', which is not used by IPA. This ends up making the
>> files more logically organized as well, since 'is_regular_file' is a
>> file operation.
>>
>> No regressions found.
>>
>> gdb/ChangeLog:
>> 2018-09-12 Sergio Durigan Junior <sergiodj@redhat.com>
>>
>> * common/common-utils.c: Don't include '<sys/stat.h>'.
>> (is_regular_file): Move to...
>> * common/filestuff.c (is_regular_file): ... here.
>> * common/common-utils.h (is_regular_file): Move to...
>> * common/filestuff.h (is_regular_file): ... here.
>
> OK.
Thanks, pushed.
3c025cfe5efc44eb4dfb03b53dca28e75096dd1e
Hi All,
Le 12/09/2018 à 19:59, Sergio Durigan Junior a écrit :
> On Wednesday, September 12 2018, Pedro Alves wrote:
>
>> On 09/12/2018 06:31 PM, Sergio Durigan Junior wrote:
>>
>>> The most simple fix for this problem is to move 'is_regular_file' to
>>> 'filestuff.c', which is not used by IPA. This ends up making the
>>> files more logically organized as well, since 'is_regular_file' is a
>>> file operation.
>>>
>>> No regressions found.
>>>
>>> gdb/ChangeLog:
>>> 2018-09-12 Sergio Durigan Junior <sergiodj@redhat.com>
>>>
>>> * common/common-utils.c: Don't include '<sys/stat.h>'.
>>> (is_regular_file): Move to...
>>> * common/filestuff.c (is_regular_file): ... here.
>>> * common/common-utils.h (is_regular_file): Move to...
>>> * common/filestuff.h (is_regular_file): ... here.
>>
>> OK.
>
> Thanks, pushed.
Thanks for the patch!
It should be backported to 8.1.1 and 8.2 release.
Best regards,
Romain
>
> 3c025cfe5efc44eb4dfb03b53dca28e75096dd1e
>
>>>>> "Romain" == Romain Naour <romain.naour@smile.fr> writes:
Romain> It should be backported to 8.1.1 and 8.2 release.
FWIW I doubt there will be another 8.1.x release.
Tom
On Friday, September 14 2018, Tom Tromey wrote:
>>>>>> "Romain" == Romain Naour <romain.naour@smile.fr> writes:
>
> Romain> It should be backported to 8.1.1 and 8.2 release.
>
> FWIW I doubt there will be another 8.1.x release.
Me too. I can backport it to 8.2 if wanted.
Thanks,
Le 14/09/2018 à 23:48, Sergio Durigan Junior a écrit :
> On Friday, September 14 2018, Tom Tromey wrote:
>
>>>>>>> "Romain" == Romain Naour <romain.naour@smile.fr> writes:
>>
>> Romain> It should be backported to 8.1.1 and 8.2 release.
>>
>> FWIW I doubt there will be another 8.1.x release.
>
> Me too. I can backport it to 8.2 if wanted.
Yes please for the upcoming 8.2.1.
Best regards,
Romain
>
> Thanks,
>
Hello Romain,
> > Me too. I can backport it to 8.2 if wanted.
>
> Yes please for the upcoming 8.2.1.
Once a release has been made off a give branch, there are a couple
of additional conditions before we can backport that change to
that branch:
- The patch needs to be safe, and you need approval from a Global
Maintainer to do so;
I looked at the patch, and although I was nervous about
the removal of the <stat.h> #include at first, I checked
the contents of the file, and I am reasonably certain that
there isn't any other code that might needed on some obscure
system. So you have my OK for the patch to be backported to
the gdb-8.2-branch.
- And the commit should have a corresponding GDB PR number
(https://sourceware.org/bugzilla/); the PR number acts as
a reference for "what's changed" in each corrective release,
so the clearer the explanation in the description, the better.
If you are the one motivated for the backport, it would be
helpful if you could take care of creating the PR. Sergio
offered to then backport it for you, but I can also help
with that.
Thank you,
Hi Joel, All,
Le 15/09/2018 à 00:09, Joel Brobecker a écrit :
> Hello Romain,
>
>>> Me too. I can backport it to 8.2 if wanted.
>>
>> Yes please for the upcoming 8.2.1.
>
> Once a release has been made off a give branch, there are a couple
> of additional conditions before we can backport that change to
> that branch:
>
> - The patch needs to be safe, and you need approval from a Global
> Maintainer to do so;
>
> I looked at the patch, and although I was nervous about
> the removal of the <stat.h> #include at first, I checked
> the contents of the file, and I am reasonably certain that
> there isn't any other code that might needed on some obscure
> system. So you have my OK for the patch to be backported to
> the gdb-8.2-branch.
Ok, thanks.
Note that Sergio is the original author of this part of the code introduced with
gdb 8.2 and backported to gdb 8.1.1 [1].
>
> - And the commit should have a corresponding GDB PR number
> (https://sourceware.org/bugzilla/); the PR number acts as
> a reference for "what's changed" in each corrective release,
> so the clearer the explanation in the description, the better.
>
> If you are the one motivated for the backport, it would be
> helpful if you could take care of creating the PR. Sergio
> offered to then backport it for you, but I can also help
> with that.
I'm agree on the principle but the change from gdb 8.2 introducing the issue has
been backported to gdb 8.1.x [1] without such GDB PR number (as far I can see).
IIUC, all patches backported to a stable branch must have a GDB PR number.
This is not always the case.
Without the fix provided by Sergio (Thanks!) we can't build gdb with a musl
toolchain, so gdb 8.1.x and gdb 8.2.x remain unfixed.
I'll create the GDB PR but all explanations of the issue have been provided on
the mailing list by gdb developers and maintainers.
[1]
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=2441702a72f324e41a1624dc042b334f375e2d81
Best regards,
Romain
>
> Thank you,
>
Le 15/09/2018 à 15:15, Romain Naour a écrit :
> Hi Joel, All,
> I'll create the GDB PR but all explanations of the issue have been provided on
> the mailing list by gdb developers and maintainers.
>
https://sourceware.org/bugzilla/show_bug.cgi?id=23663
Best regards,
Romain
On Saturday, September 15 2018, Romain Naour wrote:
>> - And the commit should have a corresponding GDB PR number
>> (https://sourceware.org/bugzilla/); the PR number acts as
>> a reference for "what's changed" in each corrective release,
>> so the clearer the explanation in the description, the better.
>>
>> If you are the one motivated for the backport, it would be
>> helpful if you could take care of creating the PR. Sergio
>> offered to then backport it for you, but I can also help
>> with that.
>
> I'm agree on the principle but the change from gdb 8.2 introducing the issue has
> been backported to gdb 8.1.x [1] without such GDB PR number (as far I can see).
> IIUC, all patches backported to a stable branch must have a GDB PR number.
> This is not always the case.
It is always the case. Joel kindly reminded me (back then) that I had
forgotten to create a PR for this backport:
https://sourceware.org/ml/gdb-patches/2018-03/msg00001.html
And I did it subsequently:
https://sourceware.org/bugzilla/show_bug.cgi?id=22907
Unfortunately, it was not possible to reference the PR number in the
commit log anymore, but it was included in the official release note:
https://sourceware.org/ml/gdb-announce/2018/msg00002.html
Thanks,
On Saturday, September 15 2018, Romain Naour wrote:
> Le 15/09/2018 à 15:15, Romain Naour a écrit :
>> Hi Joel, All,
>
>> I'll create the GDB PR but all explanations of the issue have been provided on
>> the mailing list by gdb developers and maintainers.
>>
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=23663
Thanks, I've now pushed the backported patch to the GDB 8.2 branch.
I've also set the Target Milestone of this PR to "8.2.1"; Joel, please
feel free to correct this if it is wrong.
5de69bdbd0bbd7941b4cd93d4571f5e22cdb28be
Thanks,
Hi Sergio,
Le 15/09/2018 à 22:35, Sergio Durigan Junior a écrit :
> On Saturday, September 15 2018, Romain Naour wrote:
>
>>> - And the commit should have a corresponding GDB PR number
>>> (https://sourceware.org/bugzilla/); the PR number acts as
>>> a reference for "what's changed" in each corrective release,
>>> so the clearer the explanation in the description, the better.
>>>
>>> If you are the one motivated for the backport, it would be
>>> helpful if you could take care of creating the PR. Sergio
>>> offered to then backport it for you, but I can also help
>>> with that.
>>
>> I'm agree on the principle but the change from gdb 8.2 introducing the issue has
>> been backported to gdb 8.1.x [1] without such GDB PR number (as far I can see).
>> IIUC, all patches backported to a stable branch must have a GDB PR number.
>> This is not always the case.
>
> It is always the case. Joel kindly reminded me (back then) that I had
> forgotten to create a PR for this backport:
>
> https://sourceware.org/ml/gdb-patches/2018-03/msg00001.html
>
> And I did it subsequently:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=22907
>
> Unfortunately, it was not possible to reference the PR number in the
> commit log anymore, but it was included in the official release note:
>
> https://sourceware.org/ml/gdb-announce/2018/msg00002.html
Sorry, I missed the reference to the PR.
Thanks for the links.
Best regards,
Romain
>
> Thanks,
>
On Saturday, September 15 2018, Romain Naour wrote:
> Hi Sergio,
>
> Le 15/09/2018 à 22:35, Sergio Durigan Junior a écrit :
>> On Saturday, September 15 2018, Romain Naour wrote:
>>
>>>> - And the commit should have a corresponding GDB PR number
>>>> (https://sourceware.org/bugzilla/); the PR number acts as
>>>> a reference for "what's changed" in each corrective release,
>>>> so the clearer the explanation in the description, the better.
>>>>
>>>> If you are the one motivated for the backport, it would be
>>>> helpful if you could take care of creating the PR. Sergio
>>>> offered to then backport it for you, but I can also help
>>>> with that.
>>>
>>> I'm agree on the principle but the change from gdb 8.2 introducing the issue has
>>> been backported to gdb 8.1.x [1] without such GDB PR number (as far I can see).
>>> IIUC, all patches backported to a stable branch must have a GDB PR number.
>>> This is not always the case.
>>
>> It is always the case. Joel kindly reminded me (back then) that I had
>> forgotten to create a PR for this backport:
>>
>> https://sourceware.org/ml/gdb-patches/2018-03/msg00001.html
>>
>> And I did it subsequently:
>>
>> https://sourceware.org/bugzilla/show_bug.cgi?id=22907
>>
>> Unfortunately, it was not possible to reference the PR number in the
>> commit log anymore, but it was included in the official release note:
>>
>> https://sourceware.org/ml/gdb-announce/2018/msg00002.html
>
> Sorry, I missed the reference to the PR.
> Thanks for the links.
No problem at all, it certainly wasn't obvious. Thanks for reporting
the bug and filing the PR :-).
> > I'll create the GDB PR but all explanations of the issue have been
> > provided on the mailing list by gdb developers and maintainers.
> >
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=23663
Thanks everyone. the PR number is perfect, as it provides a nice
entry point to the everything related to this issue if anyone needs
to investigate it.
@@ -20,7 +20,6 @@
#include "common-defs.h"
#include "common-utils.h"
#include "host-defs.h"
-#include <sys/stat.h>
#include <ctype.h>
/* The xmalloc() (libiberty.h) family of memory management routines.
@@ -412,37 +411,6 @@ stringify_argv (const std::vector<char *> &args)
/* See common/common-utils.h. */
-bool
-is_regular_file (const char *name, int *errno_ptr)
-{
- struct stat st;
- const int status = stat (name, &st);
-
- /* Stat should never fail except when the file does not exist.
- If stat fails, analyze the source of error and return true
- unless the file does not exist, to avoid returning false results
- on obscure systems where stat does not work as expected. */
-
- if (status != 0)
- {
- if (errno != ENOENT)
- return true;
- *errno_ptr = ENOENT;
- return false;
- }
-
- if (S_ISREG (st.st_mode))
- return true;
-
- if (S_ISDIR (st.st_mode))
- *errno_ptr = EISDIR;
- else
- *errno_ptr = EINVAL;
- return false;
-}
-
-/* See common/common-utils.h. */
-
ULONGEST
align_up (ULONGEST v, int n)
{
@@ -146,11 +146,6 @@ in_inclusive_range (T value, T low, T high)
return value >= low && value <= high;
}
-/* Return true if the file NAME exists and is a regular file.
- If the result is false then *ERRNO_PTR is set to a useful value assuming
- we're expecting a regular file. */
-extern bool is_regular_file (const char *name, int *errno_ptr);
-
/* Ensure that V is aligned to an N byte boundary (B's assumed to be a
power of 2). Round up/down when necessary. Examples of correct
use include:
@@ -417,3 +417,34 @@ make_cleanup_close (int fd)
*saved_fd = fd;
return make_cleanup_dtor (do_close_cleanup, saved_fd, xfree);
}
+
+/* See common/filestuff.h. */
+
+bool
+is_regular_file (const char *name, int *errno_ptr)
+{
+ struct stat st;
+ const int status = stat (name, &st);
+
+ /* Stat should never fail except when the file does not exist.
+ If stat fails, analyze the source of error and return true
+ unless the file does not exist, to avoid returning false results
+ on obscure systems where stat does not work as expected. */
+
+ if (status != 0)
+ {
+ if (errno != ENOENT)
+ return true;
+ *errno_ptr = ENOENT;
+ return false;
+ }
+
+ if (S_ISREG (st.st_mode))
+ return true;
+
+ if (S_ISDIR (st.st_mode))
+ *errno_ptr = EISDIR;
+ else
+ *errno_ptr = EINVAL;
+ return false;
+}
@@ -117,4 +117,9 @@ struct gdb_dir_deleter
typedef std::unique_ptr<DIR, gdb_dir_deleter> gdb_dir_up;
+/* Return true if the file NAME exists and is a regular file.
+ If the result is false then *ERRNO_PTR is set to a useful value assuming
+ we're expecting a regular file. */
+extern bool is_regular_file (const char *name, int *errno_ptr);
+
#endif /* FILESTUFF_H */