RE: [PATCH v2] legacy nfs.c: Change filesystem utime_h handler to utimens_h
Reply is below.
From: Joel Sherrill
Sent: Friday, May 21, 2021 9:44 AM
To: Gedare Bloom
Cc: Ryan Long ; vi...@rtems.rog; rtems-de...@rtems.org
Subject: Re: [PATCH v2] legacy nfs.c: Change filesystem utime_h handler to
utimens_h
As a warning, this and the corresponding libbsd patch are queued
behind an RSB patch to get UTIME_NOW and UTIME_OMIT in
newlib. That patch was merged into newlib. Then the RTEMS
*utime* patch set and these two will go in at the same time. Otherwise,
there will be breakage.
On Fri, May 21, 2021 at 9:26 AM Gedare Bloom
mailto:ged...@rtems.org>> wrote:
On Mon, May 3, 2021 at 12:41 PM Ryan Long
mailto:ryan.l...@oarcorp.com>> wrote:
>
> Changed nfs_utime() to nfs_utimens(), changed the arguments to use
> a timespec array instead of individual variables for access and
> modified time.
>
> Updates #4400
> ---
> nfsclient/src/nfs.c | 50 +-
> 1 file changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/nfsclient/src/nfs.c b/nfsclient/src/nfs.c
> index bc9a2c6..72fefb9 100644
> --- a/nfsclient/src/nfs.c
> +++ b/nfsclient/src/nfs.c
> @@ -2050,19 +2050,19 @@ static int nfs_rmnod(
> return rv;
> }
>
> -static int nfs_utime(
> +static int nfs_utimens(
> const rtems_filesystem_location_info_t *pathloc, /* IN */
> - time_t actime, /* IN */
> - time_t modtime /* IN */
> + struct timespec times[2] /* IN */
> +
> )
> {
> sattr arg;
>
> /* TODO: add rtems EPOCH - UNIX EPOCH seconds */
> - arg.atime.seconds = actime;
> - arg.atime.useconds = 0;
> - arg.mtime.seconds = modtime;
> - arg.mtime.useconds = 0;
> + arg.atime.seconds = times[0].tv_sec;
> + arg.atime.useconds = times[0].tv_nsec / 1000;
Do we have a macro exposed for this, like in
TOD_NANOSECONDS_PER_MICROSECOND? If not, this is fine I suppose.
This wouldn't be public since it doesn't start with RTEMS_ and I don't
see any of the TOD_ constants redefined in the RTEMS namespace.
> + arg.mtime.seconds = times[1].tv_sec;
> + arg.mtime.useconds = times[1].tv_nsec / 1000;
>
> return nfs_sattr(pathloc->node_access, &arg, SATTR_ATIME |
> SATTR_MTIME);
> }
> @@ -2254,25 +2254,25 @@ sattr arg;
> }
>
> const struct _rtems_filesystem_operations_table nfs_fs_ops = {
> - .lock_h = nfs_lock,
> - .unlock_h = nfs_unlock,
> - .eval_path_h= nfs_eval_path,
> - .link_h = nfs_link,
> + .lock_h= nfs_lock,
> + .unlock_h = nfs_unlock,
> + .eval_path_h = nfs_eval_path,
> + .link_h= nfs_link,
> .are_nodes_equal_h = nfs_are_nodes_equal,
> - .mknod_h= nfs_mknod,
> - .rmnod_h= nfs_rmnod,
> - .fchmod_h = nfs_fchmod,
> - .chown_h= nfs_chown,
> - .clonenod_h = nfs_clonenode,
> - .freenod_h = nfs_freenode,
> - .mount_h= rtems_filesystem_default_mount,
> - .unmount_h = rtems_filesystem_default_unmount,
> - .fsunmount_me_h = nfs_fsunmount_me,
> - .utime_h= nfs_utime,
> - .symlink_h = nfs_symlink,
> - .readlink_h = nfs_readlink,
> - .rename_h = nfs_rename,
> - .statvfs_h = rtems_filesystem_default_statvfs
> + .mknod_h = nfs_mknod,
> + .rmnod_h = nfs_rmnod,
> + .fchmod_h = nfs_fchmod,
> + .chown_h = nfs_chown,
> + .clonenod_h= nfs_clonenode,
> + .freenod_h = nfs_freenode,
> + .mount_h = rtems_filesystem_default_mount,
> + .unmount_h = rtems_filesystem_default_unmount,
> + .fsunmount_me_h= nfs_fsunmount_me,
> + .utimens_h = nfs_utimens,
> + .symlink_h = nfs_symlink,
> + .readlink_h= nfs_readlink,
> + .rename_h = nfs_rename,
> + .statvfs_h = rtems_filesystem_default_statvfs
Why so many additional whitespace changes?
[Ryan Long] When I changed utime_h to utimens_h, it got everything out of
alignment, so I moved everything over.
> };
>
> /*
> --
> 1.8.3.1
>
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH v2 3/3] main_help.c: Do not care what char is returned by getchar()
Are these three still pending? If so, can you ping the other 2 or
advise what is blocking?
On Mon, Apr 5, 2021 at 7:28 AM Ryan Long wrote:
>
> CID 1437650: Unchecked return value from library in rtems_shell_help().
>
> Closes #4291
> ---
> cpukit/libmisc/shell/main_help.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/cpukit/libmisc/shell/main_help.c
> b/cpukit/libmisc/shell/main_help.c
> index 9f59e9d..564bc30 100644
> --- a/cpukit/libmisc/shell/main_help.c
> +++ b/cpukit/libmisc/shell/main_help.c
> @@ -148,7 +148,7 @@ static int rtems_shell_help(
> line+= rtems_shell_help_cmd(shell_cmd);
>if (lines && (line > lines)) {
> printf("Press any key to continue...");
> -getchar();
> +(void) getchar();
> printf("\n");
> line = 0;
>}
> --
> 1.8.3.1
>
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
RE: [PATCH v2 3/3] main_help.c: Do not care what char is returned by getchar()
You acked them, and I sent the tarball with the patches to Joel. So I think
Joel just forgot to put them in.
-Original Message-
From: Gedare Bloom
Sent: Friday, May 28, 2021 9:55 AM
To: Ryan Long
Cc: devel@rtems.org
Subject: Re: [PATCH v2 3/3] main_help.c: Do not care what char is returned by
getchar()
Are these three still pending? If so, can you ping the other 2 or advise what
is blocking?
On Mon, Apr 5, 2021 at 7:28 AM Ryan Long wrote:
>
> CID 1437650: Unchecked return value from library in rtems_shell_help().
>
> Closes #4291
> ---
> cpukit/libmisc/shell/main_help.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/cpukit/libmisc/shell/main_help.c
> b/cpukit/libmisc/shell/main_help.c
> index 9f59e9d..564bc30 100644
> --- a/cpukit/libmisc/shell/main_help.c
> +++ b/cpukit/libmisc/shell/main_help.c
> @@ -148,7 +148,7 @@ static int rtems_shell_help(
> line+= rtems_shell_help_cmd(shell_cmd);
>if (lines && (line > lines)) {
> printf("Press any key to continue...");
> -getchar();
> +(void) getchar();
> printf("\n");
> line = 0;
>}
> --
> 1.8.3.1
>
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH v2 3/3] main_help.c: Do not care what char is returned by getchar()
Hi all,
On Fri, May 28, 2021, 08:54 Gedare Bloom wrote:
> Are these three still pending? If so, can you ping the other 2 or
> advise what is blocking?
>
> On Mon, Apr 5, 2021 at 7:28 AM Ryan Long wrote:
> >
> > CID 1437650: Unchecked return value from library in rtems_shell_help().
> >
> > Closes #4291
> > ---
> > cpukit/libmisc/shell/main_help.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/cpukit/libmisc/shell/main_help.c
> b/cpukit/libmisc/shell/main_help.c
> > index 9f59e9d..564bc30 100644
> > --- a/cpukit/libmisc/shell/main_help.c
> > +++ b/cpukit/libmisc/shell/main_help.c
> > @@ -148,7 +148,7 @@ static int rtems_shell_help(
> > line+= rtems_shell_help_cmd(shell_cmd);
> >if (lines && (line > lines)) {
> > printf("Press any key to continue...");
>
Unrelated to this patch, but shouldn't the above line say "Press ENTER to
continue"? :D
> -getchar();
> > +(void) getchar();
> > printf("\n");
> > line = 0;
> >}
> > --
> > 1.8.3.1
> >
> > ___
> > devel mailing list
> > devel@rtems.org
> > http://lists.rtems.org/mailman/listinfo/devel
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
RE: [PATCH 1/2] rtl-obj.c: Added an early return if rtl lock fails
-Original Message-
From: Chris Johns
Sent: Monday, April 5, 2021 6:46 PM
To: Ryan Long ; Gedare Bloom
Cc: devel@rtems.org
Subject: Re: [PATCH 1/2] rtl-obj.c: Added an early return if rtl lock fails
On 5/4/21 11:11 pm, Ryan Long wrote:
> When you say "allocating the main data structure", are you referring to the
> Run-Time Link Editor (rtl)? If so, rtems_rtl_data_init() is allocating memory
> for it. If it fails then rtems_rtl_data_init() returns false. Then, this
> causes rtems_rtl_lock() to return NULL.
I would need to look at this in much more detail to completely resolve what
happens at an architectural level. That is something else to this patch.
>
> -Original Message-
> From: Chris Johns
> Sent: Tuesday, March 30, 2021 11:31 PM
> To: Gedare Bloom ; Ryan Long
> Cc: devel@rtems.org
> Subject: Re: [PATCH 1/2] rtl-obj.c: Added an early return if rtl lock
> fails
>
> On 31/3/21 4:08 am, Gedare Bloom wrote:
>> This looks ok. I'm wondering however if these rtems_rtl_lock () ever
>> should fail in practice, should it be an assert? Just wondering.
>
> Good question, I cannot remember. Maybe a better solution is to move to a
> statically allocated lock using the newer interface Sebastian has added and
> removing the check?
>
> However is the lock also allocating the main data structure and also
> returning the status of that? I do this as a way of not uses resources until
> the service is used. If that is the case the change maybe OK but the name of
> the call is a bit average.
>
> Ryan, are you able to take have a closer look.
>
> Thanks
> Chris
>
>>
>> On Tue, Mar 30, 2021 at 10:58 AM Ryan Long wrote:
>>>
>>> ping
>>>
>>> -Original Message-
>>> From: Ryan Long
>>> Sent: Monday, March 22, 2021 12:08 PM
>>> To: devel@rtems.org
>>> Cc: Ryan Long
>>> Subject: [PATCH 1/2] rtl-obj.c: Added an early return if rtl lock
>>> fails
>>>
>>> CID 1444138: Dereference null return value in rtems_rtl_obj_find_file().
>>>
>>> Closes #4332
>>> ---
>>> cpukit/libdl/rtl-obj.c | 4
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/cpukit/libdl/rtl-obj.c b/cpukit/libdl/rtl-obj.c index
>>> a7dd740..d5a867e 100644
>>> --- a/cpukit/libdl/rtl-obj.c
>>> +++ b/cpukit/libdl/rtl-obj.c
>>> @@ -409,6 +409,10 @@ rtems_rtl_obj_find_file (rtems_rtl_obj* obj,
>>> const char* name)
>>>
>>>rtl = rtems_rtl_lock ();
>>>
>>> + if (rtl == NULL) {
>>> +return false;
>>> + }
An error should be set here before returning or the caller will not know whhy
that got an error.
>>> +
>>>if (!rtems_rtl_find_file (pname, rtl->paths, &obj->fname, &obj->fsize))
>>>{
>>> rtems_rtl_set_error (ENOENT, "file not found");
Here is an example.
[Ryan Long] Would this patch be good to merge with this addition? If not, what
is the preferred solution path?
Chris
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH v2] legacy nfs.c: Change filesystem utime_h handler to utimens_h
The RSB has been updated to include the newlib patch for a week now. I plan
to push the utimens patches to rtems, libbsd, and legacy by the end of
today.
I've held off a bit since this required a newlib/tool bump. I think there
has been plenty of notice now.
--joel
On Fri, May 28, 2021 at 9:14 AM Ryan Long wrote:
> Reply is below.
>
>
>
> *From:* Joel Sherrill
> *Sent:* Friday, May 21, 2021 9:44 AM
> *To:* Gedare Bloom
> *Cc:* Ryan Long ; vi...@rtems.rog;
> rtems-de...@rtems.org
> *Subject:* Re: [PATCH v2] legacy nfs.c: Change filesystem utime_h handler
> to utimens_h
>
>
>
> As a warning, this and the corresponding libbsd patch are queued
>
> behind an RSB patch to get UTIME_NOW and UTIME_OMIT in
>
> newlib. That patch was merged into newlib. Then the RTEMS
>
> *utime* patch set and these two will go in at the same time. Otherwise,
>
> there will be breakage.
>
>
>
> On Fri, May 21, 2021 at 9:26 AM Gedare Bloom wrote:
>
> On Mon, May 3, 2021 at 12:41 PM Ryan Long wrote:
> >
> > Changed nfs_utime() to nfs_utimens(), changed the arguments to use
> > a timespec array instead of individual variables for access and
> > modified time.
> >
> > Updates #4400
> > ---
> > nfsclient/src/nfs.c | 50
> +-
> > 1 file changed, 25 insertions(+), 25 deletions(-)
> >
> > diff --git a/nfsclient/src/nfs.c b/nfsclient/src/nfs.c
> > index bc9a2c6..72fefb9 100644
> > --- a/nfsclient/src/nfs.c
> > +++ b/nfsclient/src/nfs.c
> > @@ -2050,19 +2050,19 @@ static int nfs_rmnod(
> > return rv;
> > }
> >
> > -static int nfs_utime(
> > +static int nfs_utimens(
> > const rtems_filesystem_location_info_t *pathloc, /* IN */
> > - time_t actime, /* IN */
> > - time_t modtime /* IN */
> > + struct timespec times[2] /* IN */
> > +
> > )
> > {
> > sattr arg;
> >
> > /* TODO: add rtems EPOCH - UNIX EPOCH seconds */
> > - arg.atime.seconds = actime;
> > - arg.atime.useconds = 0;
> > - arg.mtime.seconds = modtime;
> > - arg.mtime.useconds = 0;
> > + arg.atime.seconds = times[0].tv_sec;
> > + arg.atime.useconds = times[0].tv_nsec / 1000;
> Do we have a macro exposed for this, like in
> TOD_NANOSECONDS_PER_MICROSECOND? If not, this is fine I suppose.
>
>
>
> This wouldn't be public since it doesn't start with RTEMS_ and I don't
>
> see any of the TOD_ constants redefined in the RTEMS namespace.
>
>
>
>
> > + arg.mtime.seconds = times[1].tv_sec;
> > + arg.mtime.useconds = times[1].tv_nsec / 1000;
> >
> > return nfs_sattr(pathloc->node_access, &arg, SATTR_ATIME |
> SATTR_MTIME);
> > }
> > @@ -2254,25 +2254,25 @@ sattr arg;
> > }
> >
> > const struct _rtems_filesystem_operations_table nfs_fs_ops = {
> > - .lock_h = nfs_lock,
> > - .unlock_h = nfs_unlock,
> > - .eval_path_h= nfs_eval_path,
> > - .link_h = nfs_link,
> > + .lock_h= nfs_lock,
> > + .unlock_h = nfs_unlock,
> > + .eval_path_h = nfs_eval_path,
> > + .link_h= nfs_link,
> > .are_nodes_equal_h = nfs_are_nodes_equal,
> > - .mknod_h= nfs_mknod,
> > - .rmnod_h= nfs_rmnod,
> > - .fchmod_h = nfs_fchmod,
> > - .chown_h= nfs_chown,
> > - .clonenod_h = nfs_clonenode,
> > - .freenod_h = nfs_freenode,
> > - .mount_h= rtems_filesystem_default_mount,
> > - .unmount_h = rtems_filesystem_default_unmount,
> > - .fsunmount_me_h = nfs_fsunmount_me,
> > - .utime_h= nfs_utime,
> > - .symlink_h = nfs_symlink,
> > - .readlink_h = nfs_readlink,
> > - .rename_h = nfs_rename,
> > - .statvfs_h = rtems_filesystem_default_statvfs
> > + .mknod_h = nfs_mknod,
> > + .rmnod_h = nfs_rmnod,
> > + .fchmod_h = nfs_fchmod,
> > + .chown_h = nfs_chown,
> > + .clonenod_h= nfs_clonenode,
> > + .freenod_h = nfs_freenode,
> > + .mount_h = rtems_filesystem_default_mount,
> > + .unmount_h = rtems_filesystem_default_unmount,
> > + .fsunmount_me_h= nfs_fsunmount_me,
> > + .utimens_h = nfs_utimens,
> > + .symlink_h = nfs_symlink,
> > + .readlink_h= nfs_readlink,
> > + .rename_h = nfs_rename,
> > + .statvfs_h = rtems_filesystem_default_statvfs
> Why so many additional whitespace changes?
>
> *[Ryan Long] When I changed utime_h to utimens_h, it got everything out of
> alignment, so I moved everything over.*
>
>
>
> > };
> >
> > /*
> > --
> > 1.8.3.1
> >
> > ___
> > devel mailing list
> > devel@rtems.org
> > http://lists
Re: [PATCH v2] cpukit/libmisc/monitor: Fix src/dest overlap of strcpy in mon-editor.c
This looks OK.
On Thu, May 27, 2021 at 7:32 PM Harrison Edward Gerber
wrote:
> From: Harrison Edward Gerber
>
> See also CID 1399727
>
> Closes #
> ---
> cpukit/libmisc/monitor/mon-editor.c | 12 +++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/cpukit/libmisc/monitor/mon-editor.c
> b/cpukit/libmisc/monitor/mon-editor.c
> index dcea9fcc69..4287b399a5 100644
> --- a/cpukit/libmisc/monitor/mon-editor.c
> +++ b/cpukit/libmisc/monitor/mon-editor.c
> @@ -360,7 +360,17 @@ rtems_monitor_line_editor (
> {
>int bs;
>pos--;
> - strcpy (buffer + pos, buffer + pos + 1);
> +
> + /*
> + * Memory operation used here instead of string
> + * method due the src and dest of buffer overlapping.
> + */
> + memmove(
> +buffer + pos,
> +buffer + pos + 1,
> +RTEMS_COMMAND_BUFFER_SIZE - pos - 1
> + );
> + buffer[RTEMS_COMMAND_BUFFER_SIZE - 1] = '\0';
>fprintf(stdout,"\b%s \b", buffer + pos);
>for (bs = 0; bs < ((int) strlen (buffer) - pos); bs++)
> putchar ('\b');
> --
> 2.25.1
>
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
[PATCH v1 1/4] rtems-fdt.c: Fix Resource leak (CID #1437645)
CID 1437645: Resource leak in rtems_fdt_load(). Closes #4297 --- cpukit/libmisc/rtems-fdt/rtems-fdt.c | 1 + 1 file changed, 1 insertion(+) diff --git a/cpukit/libmisc/rtems-fdt/rtems-fdt.c b/cpukit/libmisc/rtems-fdt/rtems-fdt.c index 7747ba9..dc5393c 100644 --- a/cpukit/libmisc/rtems-fdt/rtems-fdt.c +++ b/cpukit/libmisc/rtems-fdt/rtems-fdt.c @@ -611,6 +611,7 @@ rtems_fdt_load (const char* filename, rtems_fdt_handle* handle) return fe; } + close (bf); return 0; } -- 1.8.3.1 ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
[PATCH v1 0/4] Acked patches
Hi, These patches are from various patch sets to fix Coverity issues. These were previously acked, but other patches in the corresponding patch sets prevented them from getting pushed. Thanks, Ryan Ryan Long (4): rtems-fdt.c: Fix Resource leak (CID #1437645) main_rtrace.c: Add error return when malloc fails objectextendinformation.c: Ensure information->object_blocks is not NULL rtl-allocator.c: Put dereferences after nullcheck cpukit/libdl/rtl-allocator.c | 7 +-- cpukit/libmisc/rtems-fdt/rtems-fdt.c | 1 + cpukit/libmisc/shell/main_rtrace.c | 1 + cpukit/score/src/objectextendinformation.c | 11 +++ 4 files changed, 18 insertions(+), 2 deletions(-) -- 1.8.3.1 ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
[PATCH v1 2/4] main_rtrace.c: Add error return when malloc fails
CID 1399709: Dereference after null check in
rtems_trace_buffering_shell_save().
Closes #4329
---
cpukit/libmisc/shell/main_rtrace.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/cpukit/libmisc/shell/main_rtrace.c
b/cpukit/libmisc/shell/main_rtrace.c
index 753ab9d..e4f59c4 100644
--- a/cpukit/libmisc/shell/main_rtrace.c
+++ b/cpukit/libmisc/shell/main_rtrace.c
@@ -473,6 +473,7 @@ rtems_trace_buffering_shell_save (int argc, char *argv[])
{
close (out);
printf ("error: no memory\n");
+return 1;
}
memset (buf, 0, SAVE_BUF_SIZE);
--
1.8.3.1
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
[PATCH v1 4/4] rtl-allocator.c: Put dereferences after nullcheck
CID 1444139: Dereference null return value in rtems_rtl_alloc_hook().
Closes #4333
---
cpukit/libdl/rtl-allocator.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/cpukit/libdl/rtl-allocator.c b/cpukit/libdl/rtl-allocator.c
index 647c0c8..861754e 100644
--- a/cpukit/libdl/rtl-allocator.c
+++ b/cpukit/libdl/rtl-allocator.c
@@ -162,8 +162,11 @@ rtems_rtl_allocator
rtems_rtl_alloc_hook (rtems_rtl_allocator handler)
{
rtems_rtl_data* rtl = rtems_rtl_lock ();
- rtems_rtl_allocator previous = rtl->allocator.allocator;
- rtl->allocator.allocator = handler;
+ rtems_rtl_allocator previous = NULL;
+ if (rtl != NULL) {
+previous = rtl->allocator.allocator;
+rtl->allocator.allocator = handler;
+ }
rtems_rtl_unlock ();
return previous;
}
--
1.8.3.1
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
[PATCH v1 3/4] objectextendinformation.c: Ensure information->object_blocks is not NULL
CID 26033: Dereference after null check in _Objects_Extend_information().
Closes #4326
---
cpukit/score/src/objectextendinformation.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/cpukit/score/src/objectextendinformation.c
b/cpukit/score/src/objectextendinformation.c
index 9796eb9..c669301 100644
--- a/cpukit/score/src/objectextendinformation.c
+++ b/cpukit/score/src/objectextendinformation.c
@@ -171,6 +171,17 @@ Objects_Maximum _Objects_Extend_information(
if ( old_maximum > extend_count ) {
/*
+ * Coverity thinks there is a way for this to be NULL (CID #26033).
+ * After much time spent analyzing this, no one has identified the
+ * conditions where this can actually occur. Adding this _Assert ensures
+ * that it is never NULL. If this assert is triggered, condition
+ * generating this case will have been identified and it can be revisted.
+ * This is being done out of an abundance of caution since we could have
+ * easily flagged this as a false positive and ignored it completely.
+ */
+ _Assert(information->object_blocks != NULL);
+
+ /*
* Copy each section of the table over. This has to be performed as
* separate parts as size of each block has changed.
*/
--
1.8.3.1
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
[PATCH v1 0/6] rtems-tools resource leak patches
Hi, In these patches, I added missing fclose()'s and converted some pointers to be of the data type that they were pointing to. Thanks, Ryan Ryan Long (6): Explanations.cc: Fix resource leaks TraceReaderLogQEMU.cc: Fix resource leak GcovData.cc: Fix resource leak TraceWriterQEMU.cc: Fix resource leak DesiredSymbols.cc: Fix resource leak ReportsBase.cc: Fix Resource leak tester/covoar/DesiredSymbols.cc | 4 ++-- tester/covoar/Explanations.cc | 18 ++--- tester/covoar/GcovData.cc | 40 - tester/covoar/GcovData.h| 4 ++-- tester/covoar/ReportsBase.cc| 2 ++ tester/covoar/TraceReaderLogQEMU.cc | 3 ++- tester/covoar/TraceWriterQEMU.cc| 1 + 7 files changed, 42 insertions(+), 30 deletions(-) -- 1.8.3.1 ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
[PATCH v1 1/6] Explanations.cc: Fix resource leaks
CID 1399608: Resource leak in load().
CID 1399611: Resource leak in load().
Closes #4417
---
tester/covoar/Explanations.cc | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/tester/covoar/Explanations.cc b/tester/covoar/Explanations.cc
index 12bd809..d94cd2e 100644
--- a/tester/covoar/Explanations.cc
+++ b/tester/covoar/Explanations.cc
@@ -32,7 +32,7 @@ namespace Coverage {
#define MAX_LINE_LENGTH 512
FILE *explain;
char*cStatus;
-Explanation *e;
+Explanation e;
int line = 1;
if (!explanations)
@@ -46,7 +46,6 @@ namespace Coverage {
}
while ( 1 ) {
- e = new Explanation;
// Read the starting line of this explanation and
// skip blank lines between
do {
@@ -65,12 +64,13 @@ namespace Coverage {
what << "line " << line
<< "contains a duplicate explanation ("
<< inputBuffer << ")";
+fclose( explain );
throw rld::error( what, "Explanations::load" );
}
// Add the starting line and file
- e->startingPoint = std::string(inputBuffer);
- e->found = false;
+ e.startingPoint = std::string(inputBuffer);
+ e.found = false;
// Get the classification
cStatus = fgets( inputBuffer, MAX_LINE_LENGTH, explain );
@@ -78,10 +78,11 @@ namespace Coverage {
std::ostringstream what;
what << "line " << line
<< "out of sync at the classification";
+fclose( explain );
throw rld::error( what, "Explanations::load" );
}
inputBuffer[ strlen(inputBuffer) - 1] = '\0';
- e->classification = inputBuffer;
+ e.classification = inputBuffer;
line++;
// Get the explanation
@@ -92,6 +93,7 @@ namespace Coverage {
std::ostringstream what;
what << "line " << line
<< "out of sync at the explanation";
+ fclose( explain );
throw rld::error( what, "Explanations::load" );
}
inputBuffer[ strlen(inputBuffer) - 1] = '\0';
@@ -102,15 +104,17 @@ namespace Coverage {
break;
}
// XXX only taking last line. Needs to be a vector
-e->explanation.push_back( inputBuffer );
+e.explanation.push_back( inputBuffer );
}
// Add this to the set of Explanations
- set[ e->startingPoint ] = *e;
+ set[ e.startingPoint ] = e;
}
done:
;
+fclose( explain );
+
#undef MAX_LINE_LENGTH
}
--
1.8.3.1
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
[PATCH v1 4/6] TraceWriterQEMU.cc: Fix resource leak
CID 1399621: Resource leak in writeFile().
Closes #4420
---
tester/covoar/TraceWriterQEMU.cc | 1 +
1 file changed, 1 insertion(+)
diff --git a/tester/covoar/TraceWriterQEMU.cc b/tester/covoar/TraceWriterQEMU.cc
index 4bc9667..26447af 100644
--- a/tester/covoar/TraceWriterQEMU.cc
+++ b/tester/covoar/TraceWriterQEMU.cc
@@ -116,6 +116,7 @@ namespace Trace {
status = ::fwrite( &header, sizeof(trace_header), 1, traceFile );
if (status != 1) {
std::cerr << "Unable to write header to " << file << std::endl;
+ ::fclose( traceFile );
return false;
}
--
1.8.3.1
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
[PATCH v1 3/6] GcovData.cc: Fix resource leak
CID 1399610: Resource leak in readFrame().
Closes #4418
---
tester/covoar/GcovData.cc | 40 ++--
tester/covoar/GcovData.h | 4 ++--
2 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/tester/covoar/GcovData.cc b/tester/covoar/GcovData.cc
index 5175fb3..e8b8573 100644
--- a/tester/covoar/GcovData.cc
+++ b/tester/covoar/GcovData.cc
@@ -151,13 +151,13 @@ namespace Gcov {
fprintf( stderr, "Error while writing function announcement to a file
%s\n", gcdaFileName );
//Write function id
- buffer = (*currentFunction)->getId();
+ buffer = (*currentFunction).getId();
status = fwrite (&buffer, sizeof( buffer ), 1, gcdaFile );
if ( status != 1 )
fprintf( stderr, "Error while writing function id to a file %s\n",
gcdaFileName );
//Write function checksum
- buffer = (*currentFunction)->getChecksum();
+ buffer = (*currentFunction).getChecksum();
status = fwrite (&buffer, sizeof( buffer ), 1, gcdaFile );
if ( status != 1 )
fprintf( stderr, "Error while writing function checksum to a file
%s\n", gcdaFileName );
@@ -165,7 +165,7 @@ namespace Gcov {
// Determine how many counters there are
// and store their counts in buffer
countersFound = 0;
- (*currentFunction)->getCounters( llBuffer, countersFound, countersSum,
countersMax );
+ (*currentFunction).getCounters( llBuffer, countersFound, countersSum,
countersMax );
countersFoundSum += countersFound;
//Write info about counters
@@ -232,7 +232,6 @@ namespace Gcov {
uint32_ttempBlockId;
blocks_iterator_t tempBlockIterator;
int status;
-GcovFunctionData* newFunction;
status = readFrameHeader( &header, gcovFile);
@@ -246,13 +245,18 @@ namespace Gcov {
case GCOV_TAG_FUNCTION:
-numberOfFunctions++;
-newFunction = new GcovFunctionData;
-if ( !readFunctionFrame(header, gcovFile, newFunction) ){
- fprintf( stderr, "Error while reading FUNCTION from gcov file...\n"
);
- return false;
+{
+ numberOfFunctions++;
+ GcovFunctionData newFunction;
+
+ if ( !readFunctionFrame(header, gcovFile, &newFunction) ){
+fprintf( stderr, "Error while reading FUNCTION from gcov
file...\n" );
+return false;
+ }
+
+ functions.push_back(newFunction);
}
-functions.push_back(newFunction);
+
break;
case GCOV_TAG_BLOCKS:
@@ -269,7 +273,7 @@ namespace Gcov {
}
for( uint32_t i = 0; i < header.length; i++ )
- functions.back()->addBlock(i, intBuffer[i], "");
+ functions.back().addBlock(i, intBuffer[i], "");
break;
@@ -281,7 +285,7 @@ namespace Gcov {
}
for ( int i = 1; i < (int) header.length; i += 2 )
- functions.back()->addArc(intBuffer[0], intBuffer[i], intBuffer[i+1]);
+ functions.back().addArc(intBuffer[0], intBuffer[i], intBuffer[i+1]);
break;
@@ -299,10 +303,10 @@ namespace Gcov {
header.length -= 2;
// Find the right block
-tempBlockIterator =functions.back()->findBlockById(tempBlockId);
+tempBlockIterator =functions.back().findBlockById(tempBlockId);
header.length -= readString(buffer, gcovFile);
-functions.back()->setBlockFileName( tempBlockIterator, buffer );
+functions.back().setBlockFileName( tempBlockIterator, buffer );
status = fread( &intBuffer, 4, header.length, gcovFile );
if (status != (int) header.length){
@@ -312,7 +316,7 @@ namespace Gcov {
else
for (int i = 0; i < (int) (header.length - 2); i++)
-functions.back()->addBlockLine( tempBlockIterator, intBuffer[i] );
+functions.back().addBlockLine( tempBlockIterator, intBuffer[i] );
break;
@@ -449,8 +453,8 @@ namespace Gcov {
currentFunction++
)
{
- (*currentFunction)->printFunctionInfo( textFile, i );
- (*currentFunction)->printCoverageInfo( textFile, i );
+ (*currentFunction).printFunctionInfo( textFile, i );
+ (*currentFunction).printCoverageInfo( textFile, i );
i++;
}
@@ -497,7 +501,7 @@ namespace Gcov {
currentFunction++
)
{
- if ( !(*currentFunction)->processFunctionCounters( ) )
+ if ( !(*currentFunction).processFunctionCounters( ) )
status = false;
}
diff --git a/tester/covoar/GcovData.h b/tester/covoar/GcovData.h
index 94f5331..0e74b02 100644
--- a/tester/covoar/GcovData.h
+++ b/tester/covoar/GcovData.h
@@ -30,8 +30,8 @@ namespace Gcov {
#define GCOV_TAG_PROGRAM_SUMMARY((uint32_t)0xa300)
-typedef std::list functions_t;
-typedef std::list::iteratorfunctions_iterator_t;
+typedef std::list functions_t;
+typedef std::list::iteratorfunctions_ite
[PATCH v1 2/6] TraceReaderLogQEMU.cc: Fix resource leak
CID 1399615: Resource leak in processFile().
Closes #4419
---
tester/covoar/TraceReaderLogQEMU.cc | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tester/covoar/TraceReaderLogQEMU.cc
b/tester/covoar/TraceReaderLogQEMU.cc
index 508074a..b64a149 100644
--- a/tester/covoar/TraceReaderLogQEMU.cc
+++ b/tester/covoar/TraceReaderLogQEMU.cc
@@ -109,12 +109,12 @@ namespace Trace {
return false;
}
-
//
// Discard Header section
//
if (! ReadUntilFound( logFile, QEMU_LOG_SECTION_END ) ) {
fprintf( stderr, "Unable to locate end of log file header\n" );
+ fclose( logFile );
return false;
}
@@ -123,6 +123,7 @@ namespace Trace {
//
if (! ReadUntilFound( logFile, QEMU_LOG_IN_KEY )){
fprintf(stderr,"Error: Unable to locate first IN: Block in Log file \n");
+ fclose( logFile );
return false;
}
--
1.8.3.1
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
[PATCH v1 6/6] ReportsBase.cc: Fix Resource leak
CID 1503711: Resource leak in WriteSummaryReport(). Closes #4422 --- tester/covoar/ReportsBase.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tester/covoar/ReportsBase.cc b/tester/covoar/ReportsBase.cc index d904f8d..b4a755c 100644 --- a/tester/covoar/ReportsBase.cc +++ b/tester/covoar/ReportsBase.cc @@ -551,6 +551,8 @@ void ReportsBase::WriteSummaryReport( 100.0 - percentageBranches ); } + + fclose( report ); } void GenerateReports(const std::string& symbolSetName) -- 1.8.3.1 ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
[PATCH v1 5/6] DesiredSymbols.cc: Fix resource leak
CID 1503018: Resource leak in load().
Closes #4421
---
tester/covoar/DesiredSymbols.cc | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tester/covoar/DesiredSymbols.cc b/tester/covoar/DesiredSymbols.cc
index 2866dbe..5278891 100644
--- a/tester/covoar/DesiredSymbols.cc
+++ b/tester/covoar/DesiredSymbols.cc
@@ -101,7 +101,7 @@ namespace Coverage {
for (auto& kv : symbols.globals()) {
const rld::symbols::symbol& sym = *(kv.second);
if (sym.type() == sym.st_func) {
- set[sym.name()] = *(new SymbolInformation);
+ set[sym.name()] = SymbolInformation();
setNamesToSymbols[setName].push_back(sym.name());
}
}
@@ -109,7 +109,7 @@ namespace Coverage {
for (auto& kv : symbols.weaks()) {
const rld::symbols::symbol& sym = *(kv.second);
if (sym.type() == sym.st_func) {
- set[sym.name()] = *(new SymbolInformation);
+ set[sym.name()] = SymbolInformation();
setNamesToSymbols[setName].push_back(sym.name());
}
}
--
1.8.3.1
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH v1 0/4] Acked patches
Unless there is an objection over the weekend, I will push these Monday. Then we can begin to wade through the ones with commentary that have lingered. Probably one at a time now. --joel On Fri, May 28, 2021 at 2:11 PM Ryan Long wrote: > Hi, > > These patches are from various patch sets to fix Coverity issues. > > These were previously acked, but other patches in the corresponding > patch sets prevented them from getting pushed. > > Thanks, > Ryan > > Ryan Long (4): > rtems-fdt.c: Fix Resource leak (CID #1437645) > main_rtrace.c: Add error return when malloc fails > objectextendinformation.c: Ensure information->object_blocks is not > NULL > rtl-allocator.c: Put dereferences after nullcheck > > cpukit/libdl/rtl-allocator.c | 7 +-- > cpukit/libmisc/rtems-fdt/rtems-fdt.c | 1 + > cpukit/libmisc/shell/main_rtrace.c | 1 + > cpukit/score/src/objectextendinformation.c | 11 +++ > 4 files changed, 18 insertions(+), 2 deletions(-) > > -- > 1.8.3.1 > > ___ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel > ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH v2] cpukit/libmisc/monitor: Fix src/dest overlap of strcpy in mon-editor.c
On Fri, May 28, 2021 at 1:08 PM Joel Sherrill wrote:
>
> This looks OK.
Pushed. Thanks.
>
> On Thu, May 27, 2021 at 7:32 PM Harrison Edward Gerber
> wrote:
>>
>> From: Harrison Edward Gerber
>>
>> See also CID 1399727
>>
>> Closes #
>> ---
>> cpukit/libmisc/monitor/mon-editor.c | 12 +++-
>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/cpukit/libmisc/monitor/mon-editor.c
>> b/cpukit/libmisc/monitor/mon-editor.c
>> index dcea9fcc69..4287b399a5 100644
>> --- a/cpukit/libmisc/monitor/mon-editor.c
>> +++ b/cpukit/libmisc/monitor/mon-editor.c
>> @@ -360,7 +360,17 @@ rtems_monitor_line_editor (
>> {
>>int bs;
>>pos--;
>> - strcpy (buffer + pos, buffer + pos + 1);
>> +
>> + /*
>> + * Memory operation used here instead of string
>> + * method due the src and dest of buffer overlapping.
>> + */
>> + memmove(
>> +buffer + pos,
>> +buffer + pos + 1,
>> +RTEMS_COMMAND_BUFFER_SIZE - pos - 1
>> + );
>> + buffer[RTEMS_COMMAND_BUFFER_SIZE - 1] = '\0';
>>fprintf(stdout,"\b%s \b", buffer + pos);
>>for (bs = 0; bs < ((int) strlen (buffer) - pos); bs++)
>> putchar ('\b');
>> --
>> 2.25.1
>>
>> ___
>> devel mailing list
>> devel@rtems.org
>> http://lists.rtems.org/mailman/listinfo/devel
>
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH v1 1/6] Explanations.cc: Fix resource leaks
On 29/5/21 8:08 am, Ryan Long wrote:
> CID 1399608: Resource leak in load().
> CID 1399611: Resource leak in load().
>
> Closes #4417
> ---
> tester/covoar/Explanations.cc | 18 +++---
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/tester/covoar/Explanations.cc b/tester/covoar/Explanations.cc
> index 12bd809..d94cd2e 100644
> --- a/tester/covoar/Explanations.cc
> +++ b/tester/covoar/Explanations.cc
> @@ -32,7 +32,7 @@ namespace Coverage {
> #define MAX_LINE_LENGTH 512
> FILE *explain;
> char*cStatus;
> -Explanation *e;
> +Explanation e;
> int line = 1;
>
> if (!explanations)
> @@ -46,7 +46,6 @@ namespace Coverage {
> }
>
> while ( 1 ) {
> - e = new Explanation;
>// Read the starting line of this explanation and
>// skip blank lines between
>do {
> @@ -65,12 +64,13 @@ namespace Coverage {
> what << "line " << line
> << "contains a duplicate explanation ("
> << inputBuffer << ")";
> +fclose( explain );
> throw rld::error( what, "Explanations::load" );
If you used a std::ostream type object it would be automatically closed when
that object destructs via any exit path.
While this patch fixes the bug(s) the mixing of C here is generating these
issues and effort cleaning that up will reward you.
Chris
>}
>
>// Add the starting line and file
> - e->startingPoint = std::string(inputBuffer);
> - e->found = false;
> + e.startingPoint = std::string(inputBuffer);
> + e.found = false;
>
>// Get the classification
>cStatus = fgets( inputBuffer, MAX_LINE_LENGTH, explain );
> @@ -78,10 +78,11 @@ namespace Coverage {
> std::ostringstream what;
> what << "line " << line
> << "out of sync at the classification";
> +fclose( explain );
> throw rld::error( what, "Explanations::load" );
>}
>inputBuffer[ strlen(inputBuffer) - 1] = '\0';
> - e->classification = inputBuffer;
> + e.classification = inputBuffer;
>line++;
>
>// Get the explanation
> @@ -92,6 +93,7 @@ namespace Coverage {
>std::ostringstream what;
>what << "line " << line
> << "out of sync at the explanation";
> + fclose( explain );
>throw rld::error( what, "Explanations::load" );
> }
> inputBuffer[ strlen(inputBuffer) - 1] = '\0';
> @@ -102,15 +104,17 @@ namespace Coverage {
>break;
> }
> // XXX only taking last line. Needs to be a vector
> -e->explanation.push_back( inputBuffer );
> +e.explanation.push_back( inputBuffer );
>}
>
>// Add this to the set of Explanations
> - set[ e->startingPoint ] = *e;
> + set[ e.startingPoint ] = e;
> }
> done:
> ;
>
> +fclose( explain );
> +
> #undef MAX_LINE_LENGTH
>}
>
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH v1 4/4] rtl-allocator.c: Put dereferences after nullcheck
Ok
On 29/5/21 7:11 am, Ryan Long wrote:
> CID 1444139: Dereference null return value in rtems_rtl_alloc_hook().
>
> Closes #4333
> ---
> cpukit/libdl/rtl-allocator.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/cpukit/libdl/rtl-allocator.c b/cpukit/libdl/rtl-allocator.c
> index 647c0c8..861754e 100644
> --- a/cpukit/libdl/rtl-allocator.c
> +++ b/cpukit/libdl/rtl-allocator.c
> @@ -162,8 +162,11 @@ rtems_rtl_allocator
> rtems_rtl_alloc_hook (rtems_rtl_allocator handler)
> {
>rtems_rtl_data* rtl = rtems_rtl_lock ();
> - rtems_rtl_allocator previous = rtl->allocator.allocator;
> - rtl->allocator.allocator = handler;
> + rtems_rtl_allocator previous = NULL;
> + if (rtl != NULL) {
> +previous = rtl->allocator.allocator;
> +rtl->allocator.allocator = handler;
> + }
>rtems_rtl_unlock ();
>return previous;
> }
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
