Skip to content

ftell() not consistent in C11#8360

Open
damorinCan wants to merge 2 commits intodanmar:mainfrom
damorinCan:CWE474_ftell
Open

ftell() not consistent in C11#8360
damorinCan wants to merge 2 commits intodanmar:mainfrom
damorinCan:CWE474_ftell

Conversation

@damorinCan
Copy link
Copy Markdown

@chrchr-github
Copy link
Copy Markdown
Collaborator

Thanks for your contribution.
As far as I can tell, this has nothing to do with Windows 11, but rather the C11 spec of ftell(), right? So the title of the PR should be adjusted.
Please add a test.

@damorinCan damorinCan changed the title ftell() have a different behavior in Windows 11 than before. ftell() not consistent in C11 Mar 21, 2026
@damorinCan
Copy link
Copy Markdown
Author

So the title of the PR should be adjusted.

Title now including reference to C11 and removed reference to Windows 11.

Please add a test.

I added a test in test/testio.cpp. Anything missing in it ?

@chrchr-github
Copy link
Copy Markdown
Collaborator

Please add a test.

I added a test in test/testio.cpp. Anything missing in it ?

Thanks, I must have missed that.

@chrchr-github
Copy link
Copy Markdown
Collaborator

Ping @danmar

Copy link
Copy Markdown
Owner

@danmar danmar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel we need to have a ticket for this in trac

it feels like it should be added to the release notes as a "new check" ?

lib/checkio.cpp Outdated
case Filepointer::Operation::UNIMPORTANT:
if (f.mode == OpenMode::CLOSED)
useClosedFileError(tok);
if (isftell && windows && f.read_mode == Filepointer::ReadMode::READ_TEXT && printPortability)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the error message says something about the C11 standard. Nothing about windows.

Copy link
Copy Markdown
Author

@damorinCan damorinCan Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add a comment about this reference to this Microsoft page:

https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/ftell-ftelli64?view=msvc-170

An update for the error message with this, would it be OK ?

According to Microsoft, the value returned by ftell may not reflect the physical byte offset for streams opened in text mode, because text mode causes carriage return-line feed translation. See also 7.21.9.4 in C11 standard.".

I will remove the reference to _wfopen(), it's not part of the C++ standard.

test/testio.cpp Outdated
" (void)ftell(f);\n"
" fclose(f);\n"
" }\n"
"}\n", dinit(CheckOptions, $.platform = Platform::Type::Win32A, $.portability = true));
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test what happens on unix platform . test what happens when C99 is used - that works as expected right?

Copy link
Copy Markdown
Author

@damorinCan damorinCan Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Selecting Win32A is too limiting, it should apply to all platforms for portability.

@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants