* [patch] gdbserver <library-list> and its #FIXED version="1.0"
@ 2011-11-03 19:17 Jan Kratochvil
2015-06-07 14:11 ` ping: " Jan Kratochvil
2015-06-10 16:17 ` Pedro Alves
0 siblings, 2 replies; 6+ messages in thread
From: Jan Kratochvil @ 2011-11-03 19:17 UTC (permalink / raw)
To: gdb-patches
Hi,
while reimplementing <library-list/> I found from expat-2.0.1-11.fc15.x86_64:
warning: while parsing target library list (at line 1): Required attribute "version" of <library-list-svr4> not specified
I believe the same bug has to apply for existing FSF gdbserver but I do not
have any <library-list/> platform to test it (I did not try to build MinGW).
features/library-list.dtd:
<!ATTLIST library-list version CDATA #FIXED "1.0">
http://www.xml.com/pub/a/98/10/guide0.html?page=3 says:
#FIXED
In this case, the attribute is not required, but if it occurs, it must
have the specified value.
Which would suggest gdbserver is right but solib-target.c is wrong. One could
also make gdbserver explicit for the version (if those 14 bytes are not of
a concern).
Not going to check it in without a comment/approval.
Thanks,
Jan
gdb/
2011-11-03 Jan Kratochvil <jan.kratochvil@redhat.com>
* solib-target.c (library_list_start_list): Do not dereference
variable version in its initialization. Make the VERSION check handle
NULL.
(library_list_attributes): Make "version" GDB_XML_AF_OPTIONAL.
gdb/gdbserver/
2011-11-03 Jan Kratochvil <jan.kratochvil@redhat.com>
* server.c (handle_qxfer_libraries): Set `version' attribute for
<library-list>.
--- a/gdb/solib-target.c
+++ b/gdb/solib-target.c
@@ -150,12 +150,18 @@ library_list_start_list (struct gdb_xml_parser *parser,
const struct gdb_xml_element *element,
void *user_data, VEC(gdb_xml_value_s) *attributes)
{
- char *version = xml_find_attribute (attributes, "version")->value;
+ struct gdb_xml_value *version = xml_find_attribute (attributes, "version");
- if (strcmp (version, "1.0") != 0)
- gdb_xml_error (parser,
- _("Library list has unsupported version \"%s\""),
- version);
+ /* #FIXED attribute may be omitted, Expat returns NULL in such case. */
+ if (version)
+ {
+ const char *string = version->value;
+
+ if (strcmp (string, "1.0") != 0)
+ gdb_xml_error (parser,
+ _("Library list has unsupported version \"%s\""),
+ version);
+ }
}
/* Discard the constructed library list. */
@@ -214,7 +220,7 @@ static const struct gdb_xml_element library_list_children[] = {
};
static const struct gdb_xml_attribute library_list_attributes[] = {
- { "version", GDB_XML_AF_NONE, NULL, NULL },
+ { "version", GDB_XML_AF_OPTIONAL, NULL, NULL },
{ NULL, GDB_XML_AF_NONE, NULL, NULL }
};
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -952,7 +952,7 @@ handle_qxfer_libraries (const char *annex,
if (document == NULL)
return -1;
- strcpy (document, "<library-list>\n");
+ strcpy (document, "<library-list version=\"1.0\">\n");
p = document + strlen (document);
for (dll_ptr = all_dlls.head; dll_ptr != NULL; dll_ptr = dll_ptr->next)
^ permalink raw reply [flat|nested] 6+ messages in thread
* ping: [patch] gdbserver <library-list> and its #FIXED version="1.0"
2011-11-03 19:17 [patch] gdbserver <library-list> and its #FIXED version="1.0" Jan Kratochvil
@ 2015-06-07 14:11 ` Jan Kratochvil
2015-06-08 9:08 ` Gary Benson
2015-06-10 16:17 ` Pedro Alves
1 sibling, 1 reply; 6+ messages in thread
From: Jan Kratochvil @ 2015-06-07 14:11 UTC (permalink / raw)
To: gdb-patches
Hi,
this patch was never replied:
[patch] gdbserver <library-list> and its #FIXED version="1.0"
https://sourceware.org/ml/gdb-patches/2011-11/msg00099.html
Message-ID: <20111103191640.GA27298@host1.jankratochvil.net>
but in this patch Tom Tromey has IIUC approved it:
Re: [PATCH v5 6/8] gdbserver build-id attribute generator
https://sourceware.org/ml/gdb-patches/2014-05/msg00490.html
Message-ID: <87lhtvm634.fsf@fleche.redhat.com>
I guess I should check it in the first referenced patch but given all the years
have passed and maintainers changed I am rather pinging it for a new opinion.
Jan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: ping: [patch] gdbserver <library-list> and its #FIXED version="1.0"
2015-06-07 14:11 ` ping: " Jan Kratochvil
@ 2015-06-08 9:08 ` Gary Benson
2015-06-10 16:31 ` [commit] " Jan Kratochvil
0 siblings, 1 reply; 6+ messages in thread
From: Gary Benson @ 2015-06-08 9:08 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
Jan Kratochvil wrote:
> this patch was never replied:
> [patch] gdbserver <library-list> and its #FIXED version="1.0"
> https://sourceware.org/ml/gdb-patches/2011-11/msg00099.html
> Message-ID: <20111103191640.GA27298@host1.jankratochvil.net>
>
> but in this patch Tom Tromey has IIUC approved it:
> Re: [PATCH v5 6/8] gdbserver build-id attribute generator
> https://sourceware.org/ml/gdb-patches/2014-05/msg00490.html
> Message-ID: <87lhtvm634.fsf@fleche.redhat.com>
>
> I guess I should check it in the first referenced patch but given all the years
> have passed and maintainers changed I am rather pinging it for a new opinion.
The "if (version)" should likely be "if (version != NULL)".
Otherwise it seems ok.
Cheers,
Gary
--
http://gbenson.net/
^ permalink raw reply [flat|nested] 6+ messages in thread
* [commit] [patch] gdbserver <library-list> and its #FIXED version="1.0"
2015-06-08 9:08 ` Gary Benson
@ 2015-06-10 16:31 ` Jan Kratochvil
2015-06-10 17:38 ` [commit#2] " Jan Kratochvil
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kratochvil @ 2015-06-10 16:31 UTC (permalink / raw)
To: Gary Benson, Pedro Alves; +Cc: gdb-patches
On Mon, 08 Jun 2015 11:07:56 +0200, Gary Benson wrote:
> The "if (version)" should likely be "if (version != NULL)".
On Wed, 10 Jun 2015 18:17:43 +0200, Pedro Alves wrote:
> Otherwise looks fine.
Checked in:
24c05f46059182f0c8768c6ebbb66b4ca3233ecc
Jan
^ permalink raw reply [flat|nested] 6+ messages in thread
* [commit#2] [patch] gdbserver <library-list> and its #FIXED version="1.0"
2015-06-10 16:31 ` [commit] " Jan Kratochvil
@ 2015-06-10 17:38 ` Jan Kratochvil
0 siblings, 0 replies; 6+ messages in thread
From: Jan Kratochvil @ 2015-06-10 17:38 UTC (permalink / raw)
To: Gary Benson, Pedro Alves; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 236 bytes --]
On Wed, 10 Jun 2015 18:31:21 +0200, Jan Kratochvil wrote:
> On Mon, 08 Jun 2015 11:07:56 +0200, Gary Benson wrote:
> > The "if (version)" should likely be "if (version != NULL)".
I forgot about this part. Checked in as obvious.
Jan
[-- Attachment #2: Type: message/rfc822, Size: 1402 bytes --]
From: Jan Kratochvil <jan.kratochvil@redhat.com>
Subject: [PATCH] Code cleanup: Use explicit NULL comparison
Date: Wed, 10 Jun 2015 19:37:19 +0200
gdb/ChangeLog
2015-06-10 Jan Kratochvil <jan.kratochvil@redhat.com>
Code cleanup.
* solib-target.c (library_list_start_list): Use explicit NULL
comparison.
---
gdb/ChangeLog | 6 ++++++
gdb/solib-target.c | 2 +-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 83845f4..5b19052 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
2015-06-10 Jan Kratochvil <jan.kratochvil@redhat.com>
+ Code cleanup.
+ * solib-target.c (library_list_start_list): Use explicit NULL
+ comparison.
+
+2015-06-10 Jan Kratochvil <jan.kratochvil@redhat.com>
+
* solib-target.c (library_list_start_list): Do not dereference
variable version in its initialization. Make the VERSION check handle
NULL.
diff --git a/gdb/solib-target.c b/gdb/solib-target.c
index 891e572..f14363a 100644
--- a/gdb/solib-target.c
+++ b/gdb/solib-target.c
@@ -149,7 +149,7 @@ library_list_start_list (struct gdb_xml_parser *parser,
struct gdb_xml_value *version = xml_find_attribute (attributes, "version");
/* #FIXED attribute may be omitted, Expat returns NULL in such case. */
- if (version)
+ if (version != NULL)
{
const char *string = version->value;
--
2.1.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] gdbserver <library-list> and its #FIXED version="1.0"
2011-11-03 19:17 [patch] gdbserver <library-list> and its #FIXED version="1.0" Jan Kratochvil
2015-06-07 14:11 ` ping: " Jan Kratochvil
@ 2015-06-10 16:17 ` Pedro Alves
1 sibling, 0 replies; 6+ messages in thread
From: Pedro Alves @ 2015-06-10 16:17 UTC (permalink / raw)
To: Jan Kratochvil, gdb-patches
On 11/03/2011 07:16 PM, Jan Kratochvil wrote:
> I believe the same bug has to apply for existing FSF gdbserver but I do not
> have any <library-list/> platform to test it (I did not try to build MinGW).
>
> features/library-list.dtd:
> <!ATTLIST library-list version CDATA #FIXED "1.0">
>
> http://www.xml.com/pub/a/98/10/guide0.html?page=3 says:
>
> #FIXED
> In this case, the attribute is not required, but if it occurs, it must
> have the specified value.
>
> Which would suggest gdbserver is right but solib-target.c is wrong. One could
> also make gdbserver explicit for the version (if those 14 bytes are not of
> a concern).
Yeah, I couldn't find any other place that handled "version"
optionally. Looking around, it looks like we end up passing
an explicit version number for all xml files but this one.
> + /* #FIXED attribute may be omitted, Expat returns NULL in such case. */
> + if (version)
version != NULL
Otherwise looks fine.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-06-10 17:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-11-03 19:17 [patch] gdbserver <library-list> and its #FIXED version="1.0" Jan Kratochvil
2015-06-07 14:11 ` ping: " Jan Kratochvil
2015-06-08 9:08 ` Gary Benson
2015-06-10 16:31 ` [commit] " Jan Kratochvil
2015-06-10 17:38 ` [commit#2] " Jan Kratochvil
2015-06-10 16:17 ` Pedro Alves
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).