* Missing NULL check
@ 2023-09-12 15:40 jacob navia
2023-09-12 18:50 ` Torbjorn SVENSSON
2023-09-13 6:50 ` Alan Modra
0 siblings, 2 replies; 6+ messages in thread
From: jacob navia @ 2023-09-12 15:40 UTC (permalink / raw)
To: binutils
[-- Attachment #1: Type: text/plain, Size: 1074 bytes --]
This is very similar to the last one:
FILE: elf-attrs.c line 232
FUNCTION: elf_new_obj_attr
/* Allocate/find an object attribute. */
static obj_attribute *
elf_new_obj_attr (bfd *abfd, int vendor, unsigned int tag)
{
obj_attribute *attr;
obj_attribute_list *list;
obj_attribute_list *p;
obj_attribute_list **lastp;
if (tag < NUM_KNOWN_OBJ_ATTRIBUTES)
{
/* Known tags are preallocated. */
attr = &elf_known_obj_attributes (abfd)[vendor][tag];
}
else
{
/* Create a new tag. */
list = (obj_attribute_list *)
bfd_alloc (abfd, sizeof (obj_attribute_list));
memset (list, 0, sizeof (obj_attribute_list));
list->tag = tag;
/* Keep the tag list in order. */
lastp = &elf_other_obj_attributes (abfd)[vendor];
for (p = *lastp; p; p = p->next)
{
if (tag < p->tag)
break;
lastp = &p->next;
}
list->next = *lastp;
*lastp = list;
attr = &list->attr;
}
return attr;
}
bfd_alloc can return NULL. This is not checked
How to fix:
Add
if (list == NULL) return NULL;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Missing NULL check
2023-09-12 15:40 Missing NULL check jacob navia
@ 2023-09-12 18:50 ` Torbjorn SVENSSON
2023-09-12 19:34 ` Fangrui Song
[not found] ` <DS7PR12MB5765DD3A03E4B143B950C3D6CBF1A@DS7PR12MB5765.namprd12.prod.outlook.com>
2023-09-13 6:50 ` Alan Modra
1 sibling, 2 replies; 6+ messages in thread
From: Torbjorn SVENSSON @ 2023-09-12 18:50 UTC (permalink / raw)
To: jacob navia, binutils
On 2023-09-12 17:40, jacob navia wrote:
> This is very similar to the last one:
> FILE: elf-attrs.c line 232
> FUNCTION: elf_new_obj_attr
> /* Allocate/find an object attribute. */
> static obj_attribute *
> elf_new_obj_attr (bfd *abfd, int vendor, unsigned int tag)
> {
> obj_attribute *attr;
> obj_attribute_list *list;
> obj_attribute_list *p;
> obj_attribute_list **lastp;
>
>
> if (tag < NUM_KNOWN_OBJ_ATTRIBUTES)
> {
> /* Known tags are preallocated. */
> attr = &elf_known_obj_attributes (abfd)[vendor][tag];
> }
> else
> {
> /* Create a new tag. */
> list = (obj_attribute_list *)
> bfd_alloc (abfd, sizeof (obj_attribute_list));
> memset (list, 0, sizeof (obj_attribute_list));
> list->tag = tag;
> /* Keep the tag list in order. */
> lastp = &elf_other_obj_attributes (abfd)[vendor];
> for (p = *lastp; p; p = p->next)
> {
> if (tag < p->tag)
> break;
> lastp = &p->next;
> }
> list->next = *lastp;
> *lastp = list;
> attr = &list->attr;
> }
>
> return attr;
> }
>
> bfd_alloc can return NULL. This is not checked
> How to fix:
> Add
> if (list == NULL) return NULL;
I've seen that you've sent a few mails similar to this one now.
First of all, thanks for trying to improve the quality on binutils!
If you want to help binutils even more, I would suggest that you create
a proper patch and submit that instead of a mail with the above content
as it would make it less work for a maintainer to actually commit your fix.
Note, the above is just my personal opinion and might not be align with
the preferences of the binutils maintainers.
Kind regards,
Torbjörn
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Missing NULL check
2023-09-12 18:50 ` Torbjorn SVENSSON
@ 2023-09-12 19:34 ` Fangrui Song
2023-09-13 10:57 ` Nick Clifton
[not found] ` <DS7PR12MB5765DD3A03E4B143B950C3D6CBF1A@DS7PR12MB5765.namprd12.prod.outlook.com>
1 sibling, 1 reply; 6+ messages in thread
From: Fangrui Song @ 2023-09-12 19:34 UTC (permalink / raw)
To: jacob navia; +Cc: Torbjorn SVENSSON, binutils
On Tue, Sep 12, 2023 at 11:50 AM Torbjorn SVENSSON via Binutils
<binutils@sourceware.org> wrote:
>
>
>
> On 2023-09-12 17:40, jacob navia wrote:
> > This is very similar to the last one:
> > FILE: elf-attrs.c line 232
> > FUNCTION: elf_new_obj_attr
> > /* Allocate/find an object attribute. */
> > static obj_attribute *
> > elf_new_obj_attr (bfd *abfd, int vendor, unsigned int tag)
> > {
> > obj_attribute *attr;
> > obj_attribute_list *list;
> > obj_attribute_list *p;
> > obj_attribute_list **lastp;
> >
> >
> > if (tag < NUM_KNOWN_OBJ_ATTRIBUTES)
> > {
> > /* Known tags are preallocated. */
> > attr = &elf_known_obj_attributes (abfd)[vendor][tag];
> > }
> > else
> > {
> > /* Create a new tag. */
> > list = (obj_attribute_list *)
> > bfd_alloc (abfd, sizeof (obj_attribute_list));
> > memset (list, 0, sizeof (obj_attribute_list));
> > list->tag = tag;
> > /* Keep the tag list in order. */
> > lastp = &elf_other_obj_attributes (abfd)[vendor];
> > for (p = *lastp; p; p = p->next)
> > {
> > if (tag < p->tag)
> > break;
> > lastp = &p->next;
> > }
> > list->next = *lastp;
> > *lastp = list;
> > attr = &list->attr;
> > }
> >
> > return attr;
> > }
> >
> > bfd_alloc can return NULL. This is not checked
> > How to fix:
> > Add
> > if (list == NULL) return NULL;
>
> I've seen that you've sent a few mails similar to this one now.
> First of all, thanks for trying to improve the quality on binutils!
>
> If you want to help binutils even more, I would suggest that you create
> a proper patch and submit that instead of a mail with the above content
> as it would make it less work for a maintainer to actually commit your fix.
>
> Note, the above is just my personal opinion and might not be align with
> the preferences of the binutils maintainers.
>
> Kind regards,
> Torbjörn
I share the same opinion as Torbjorn's.
You might consider configuring `git send-email` with a MTA (e.g.
msmtp), then you can send [PATCH] messages that a maintainer can apply
with ease:
b4 am 'https://inbox.sourceware.org/binutils/DB9PR08MB647413BBA3340351A802035F81F1A@DB9PR08MB6474.eurprd08.prod.outlook.com/T/#t'
-o - | git am
(See https://josefbacik.github.io/kernel/2021/10/18/lei-and-b4.html)
If a patch is straightforward enough and a maintainer notices it, I am
sure this will make it faster for a fix to be applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Missing NULL check
2023-09-12 15:40 Missing NULL check jacob navia
2023-09-12 18:50 ` Torbjorn SVENSSON
@ 2023-09-13 6:50 ` Alan Modra
1 sibling, 0 replies; 6+ messages in thread
From: Alan Modra @ 2023-09-13 6:50 UTC (permalink / raw)
To: jacob navia; +Cc: binutils
On Tue, Sep 12, 2023 at 05:40:45PM +0200, jacob navia wrote:
> Add
> if (list == NULL) return NULL;
Yes, we should be testing for a NULL return from bfd_alloc, but error
handling then needs to be propagated up the call chain. If
elf_new_obj_attr returns NULL then all its callers will segfault
unless they are changed too. We also don't want to hide allocation
failures which means functions like bfd_elf_add_obj_attr_int should
return the obj_attribute pointer rather than void, and all callers
should check the return value. I'll write a patch to do that.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Missing NULL check
2023-09-12 19:34 ` Fangrui Song
@ 2023-09-13 10:57 ` Nick Clifton
0 siblings, 0 replies; 6+ messages in thread
From: Nick Clifton @ 2023-09-13 10:57 UTC (permalink / raw)
To: jacob navia; +Cc: binutils
Hi Jacob,
Following on from Torbjorn's and Fanguri's points, I would also like
to say that for non-trial bugs, or code improvements, it really helps
if you file a report via the sourceware bugzilla system:
https://sourceware.org/bugzilla/enter_bug.cgi?product=binutils
This allows us to keep all of the discussion related to the issue in
one place, and to refer to it in the code, git-log entries and changelog
entries via a simple problem report number.
Cheers
Nick
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Missing NULL check
[not found] ` <DS7PR12MB5765DD3A03E4B143B950C3D6CBF1A@DS7PR12MB5765.namprd12.prod.outlook.com>
@ 2023-09-13 14:28 ` jacob navia
0 siblings, 0 replies; 6+ messages in thread
From: jacob navia @ 2023-09-13 14:28 UTC (permalink / raw)
To: Fangrui Song; +Cc: Torbjorn SVENSSON, binutils
[-- Attachment #1: Type: text/plain, Size: 7014 bytes --]
>
> I share the same opinion as Torbjorn's.
>
> You might consider configuring `git send-email` with a MTA (e.g.
> msmtp), then you can send [PATCH] messages that a maintainer can apply
> with ease:
>
> b4 am 'https://inbox.sourceware.org/binutils/DB9PR08MB647413BBA3340351A802035F81F1A@DB9PR08MB6474.eurprd08.prod.outlook.com/T/#t'
> -o - | git am
>
> (See https://josefbacik.github.io/kernel/2021/10/18/lei-and-b4.html)
>
> If a patch is straightforward enough and a maintainer notices it, I am
> sure this will make it faster for a fix to be applied.
This looks easy for linux wizards… I am not. I am just trying to help.
1) I downloaded and installed smtp
2) Download and installed the « b4 » thing.
3) Downloaded and installed a fresh copy of binutils-gdb
4) Made the change for fixing missing NULL tests.
5) Committed my changes to git
6) After a lot of fiddling I did the command that you sent me:
b4 am 'https://inbox.sourceware.org/binutils/DB9PR08MB647413BBA3340351A802035F81F1A@DB9PR08MB6474.eurprd08.prod.outlook.com/T/#t'
-o - | git am
It was splitter somewhere, did not work the first few times until I got that right.
7) OUTPUT:
sipeed@lpi4a:~/binutils-gdb/bfd$ b4 am 'https://inbox.sourceware.org/binutils/DB9PR08MB647413BBA3340351A802035F81F1A@DB9PR08MB6474.eurprd08.prod.outlook.com/T/#t' -o - | git am
Looking up https://inbox.sourceware.org/binutils/DB9PR08MB647413BBA3340351A802035F81F1A%40DB9PR08MB6474.eurprd08.prod.outlook.com
Grabbing thread from inbox.sourceware.org/binutils/DB9PR08MB647413BBA3340351A802035F81F1A%40DB9PR08MB6474.eurprd08.prod.outlook.com/t.mbox.gz
Traceback (most recent call last):
File "/usr/lib/python3/dist-packages/urllib3/connectionpool.py", line 704, in urlopen
httplib_response = self._make_request(
^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3/dist-packages/urllib3/connectionpool.py", line 450, in _make_request
six.raise_from(e, None)
File "<string>", line 3, in raise_from
File "/usr/lib/python3/dist-packages/urllib3/connectionpool.py", line 445, in _make_request
httplib_response = conn.getresponse()
^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/http/client.py", line 1378, in getresponse
response.begin()
File "/usr/lib/python3.11/http/client.py", line 318, in begin
version, status, reason = self._read_status()
^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/http/client.py", line 287, in _read_status
raise RemoteDisconnected("Remote end closed connection without"
http.client.RemoteDisconnected: Remote end closed connection without response
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/usr/lib/python3/dist-packages/requests/adapters.py", line 489, in send
resp = conn.urlopen(
^^^^^^^^^^^^^
File "/usr/lib/python3/dist-packages/urllib3/connectionpool.py", line 788, in urlopen
retries = retries.increment(
^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3/dist-packages/urllib3/util/retry.py", line 550, in increment
raise six.reraise(type(error), error, _stacktrace)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3/dist-packages/six.py", line 718, in reraise
raise value.with_traceback(tb)
File "/usr/lib/python3/dist-packages/urllib3/connectionpool.py", line 704, in urlopen
httplib_response = self._make_request(
^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3/dist-packages/urllib3/connectionpool.py", line 450, in _make_request
six.raise_from(e, None)
File "<string>", line 3, in raise_from
File "/usr/lib/python3/dist-packages/urllib3/connectionpool.py", line 445, in _make_request
httplib_response = conn.getresponse()
^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/http/client.py", line 1378, in getresponse
response.begin()
File "/usr/lib/python3.11/http/client.py", line 318, in begin
version, status, reason = self._read_status()
^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/http/client.py", line 287, in _read_status
raise RemoteDisconnected("Remote end closed connection without"
urllib3.exceptions.ProtocolError: ('Connection aborted.', RemoteDisconnected('Remote end closed connection without response'))
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/usr/bin/b4", line 33, in <module>
sys.exit(load_entry_point('b4==0.12.2', 'console_scripts', 'b4')())
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3/dist-packages/b4/command.py", line 360, in cmd
cmdargs.func(cmdargs)
File "/usr/lib/python3/dist-packages/b4/command.py", line 91, in cmd_am
b4.mbox.main(cmdargs)
File "/usr/lib/python3/dist-packages/b4/mbox.py", line 694, in main
msgid, msgs = b4.retrieve_messages(cmdargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3/dist-packages/b4/__init__.py", line 3538, in retrieve_messages
msgs = get_pi_thread_by_msgid(msgid, nocache=cmdargs.nocache, onlymsgids=pickings)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3/dist-packages/b4/__init__.py", line 2843, in get_pi_thread_by_msgid
msgs = get_pi_thread_by_url(t_mbx_url, nocache=nocache)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3/dist-packages/b4/__init__.py", line 2800, in get_pi_thread_by_url
resp = session.get(t_mbx_url)
^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3/dist-packages/requests/sessions.py", line 600, in get
return self.request("GET", url, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3/dist-packages/requests/sessions.py", line 587, in request
resp = self.send(prep, **send_kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3/dist-packages/requests/sessions.py", line 701, in send
r = adapter.send(request, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3/dist-packages/requests/adapters.py", line 547, in send
raise ConnectionError(err, request=request)
requests.exceptions.ConnectionError: ('Connection aborted.', RemoteDisconnected('Remote end closed connection without response'))
sipeed@lpi4a:~/binutils-gdb/bfd$
Look, I am not a linux wizard, nor a python wizard, and neither a git wizard. From the first lines the command seem to work since it says:
Grabbing thread from inbox.sourceware.org/binutils/DB9PR08MB647413BBA3340351A802035F81F1A%40DB9PR08MB6474.eurprd08.prod.outlook.com/t.mbox.gz <http://inbox.sourceware.org/binutils/DB9PR08MB647413BBA3340351A802035F81F1A%40DB9PR08MB6474.eurprd08.prod.outlook.com/t.mbox.gz>
Afterwards, please debug that yourself…
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-09-13 14:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-12 15:40 Missing NULL check jacob navia
2023-09-12 18:50 ` Torbjorn SVENSSON
2023-09-12 19:34 ` Fangrui Song
2023-09-13 10:57 ` Nick Clifton
[not found] ` <DS7PR12MB5765DD3A03E4B143B950C3D6CBF1A@DS7PR12MB5765.namprd12.prod.outlook.com>
2023-09-13 14:28 ` jacob navia
2023-09-13 6:50 ` Alan Modra
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).