Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix up pointer subtraction (do not cast to integers) #142

Closed
skliper opened this issue Sep 30, 2019 · 5 comments
Closed

Fix up pointer subtraction (do not cast to integers) #142

skliper opened this issue Sep 30, 2019 · 5 comments
Labels
Milestone

Comments

@skliper
Copy link
Contributor

skliper commented Sep 30, 2019

Pointer subtraction works. Code that casts the pointers
into integer types differs from the correct code only in
its obfuscation.

Additionally, where pointer subtraction is used to determine
the length of a string, the entire sequence should be replaced
by a call to strlen().

On a side note, dividing by "sizeof(char)" is also mere obfuscation
as the standard defines that sizeof returns the size in (char) units.

Note that when attempting to compile for 64-bit targets, these will usually be flagged as "casting pointer to wrong sized integer" -- this may help to find problematic code.

Start with OS_check_name_length().

@skliper skliper added this to the osal-4.2 milestone Sep 30, 2019
@skliper skliper self-assigned this Sep 30, 2019
@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Imported from trac issue 119. Created by glimes on 2015-11-20T14:49:27, last modified: 2015-12-09T11:13:56

@skliper skliper added the bug label Sep 30, 2019
@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by glimes on 2015-11-30 15:20:49:

I could have sworn I saw this sort of thing in several places, but the scan of the OSAL development tree today only picked up the three OS_check_name_length() implementations (posix, vxworks, rtems).

Original code:
{{{
name_ptr = strrchr(path, '/');
if (name_ptr == NULL)
return OS_FS_ERROR;

name_ptr = name_ptr + sizeof(char);

end_of_path = strrchr(path,'\0');

name_len = ((int) end_of_path - (int)name_ptr) / sizeof(char);

if( name_len > OS_MAX_FILE_NAME)
    return OS_FS_ERROR;

}}}

Replacement code:
{{{
name_ptr = strrchr(path, '/');
if (name_ptr == NULL)
return OS_FS_ERROR;

name_ptr ++;

if( strlen(name_ptr) > OS_MAX_FILE_NAME)
    return OS_FS_ERROR;

}}}

In both cases, comments elided and braces minimized for brevity in this ticket.

This change has been pushed to babelfish as [changeset:90d6145 changeset 90d6145: trac-119-bad-pointer-math]

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by glimes on 2015-12-01 11:18:12:

Ephemeral test links from last night (these go away soon):

[https://babelfish.arc.nasa.gov/bamboo/browse/CFS-CFSOSAL54-241 OSAL ic-gll-merge results look good]

[https://babelfish.arc.nasa.gov/bamboo/browse/CFS-CFSCFE47-78 CFE ic-gll-merge results look good]

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by glimes on 2015-12-09 11:04:43:

Commit [changeset:90d6145] included via merge [changeset:f3ebb35] into merge [changeset:961b061] which is now the development branch.

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by glimes on 2015-12-09 11:13:56:

Merge of #136 #138 #139 #141 and #142 (2015-12-01) is now development.

@skliper skliper closed this as completed Sep 30, 2019
@skliper skliper removed their assignment Sep 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant