Skip to content

Commit bd6a132

Browse files
committed
New check: ftell() result is unspecified when file is opened in mode "t".
New changes: - Added missing string related to new check - Added checker description for ftellTextModeFile - Updated releasenotes.txt with new check
1 parent 72547b3 commit bd6a132

6 files changed

Lines changed: 97 additions & 0 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
*.gcno
33
*.gch
44
*.o
5+
*.a
56
*.pyc
67
/cppcheck
78
/cppcheck.exe

lib/checkio.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
// CVE ID used:
4949
static const CWE CWE119(119U); // Improper Restriction of Operations within the Bounds of a Memory Buffer
5050
static const CWE CWE398(398U); // Indicator of Poor Code Quality
51+
static const CWE CWE474(474U); // Use of Function with Inconsistent Implementations
5152
static const CWE CWE664(664U); // Improper Control of a Resource Through its Lifetime
5253
static const CWE CWE685(685U); // Function Call With Incorrect Number of Arguments
5354
static const CWE CWE686(686U); // Function Call With Incorrect Argument Type
@@ -111,6 +112,8 @@ namespace {
111112
nonneg int op_indent{};
112113
enum class AppendMode : std::uint8_t { UNKNOWN_AM, APPEND, APPEND_EX };
113114
AppendMode append_mode = AppendMode::UNKNOWN_AM;
115+
enum class ReadMode : std::uint8_t { READ_TEXT, READ_BIN };
116+
ReadMode read_mode = ReadMode::READ_BIN;
114117
std::string filename;
115118
explicit Filepointer(OpenMode mode_ = OpenMode::UNKNOWN_OM)
116119
: mode(mode_) {}
@@ -183,6 +186,7 @@ void CheckIO::checkFileUsage()
183186
}
184187
} else if (Token::Match(tok, "%name% (") && tok->previous() && (!tok->previous()->isName() || Token::Match(tok->previous(), "return|throw"))) {
185188
std::string mode;
189+
bool isftell = false;
186190
const Token* fileTok = nullptr;
187191
const Token* fileNameTok = nullptr;
188192
Filepointer::Operation operation = Filepointer::Operation::NONE;
@@ -266,6 +270,9 @@ void CheckIO::checkFileUsage()
266270
fileTok = tok->tokAt(2);
267271
if ((tok->str() == "ungetc" || tok->str() == "ungetwc") && fileTok)
268272
fileTok = fileTok->nextArgument();
273+
else if (tok->str() == "ftell") {
274+
isftell = true;
275+
}
269276
operation = Filepointer::Operation::UNIMPORTANT;
270277
} else if (!Token::Match(tok, "if|for|while|catch|switch") && !mSettings->library.isFunctionConst(tok->str(), true)) {
271278
const Token* const end2 = tok->linkAt(1);
@@ -321,10 +328,15 @@ void CheckIO::checkFileUsage()
321328
f.append_mode = Filepointer::AppendMode::APPEND_EX;
322329
else
323330
f.append_mode = Filepointer::AppendMode::APPEND;
331+
}
332+
else if (mode.find('r') != std::string::npos &&
333+
mode.find('t') != std::string::npos) {
334+
f.read_mode = Filepointer::ReadMode::READ_TEXT;
324335
} else
325336
f.append_mode = Filepointer::AppendMode::UNKNOWN_AM;
326337
f.mode_indent = indent;
327338
break;
339+
328340
case Filepointer::Operation::POSITIONING:
329341
if (f.mode == OpenMode::CLOSED)
330342
useClosedFileError(tok);
@@ -357,6 +369,8 @@ void CheckIO::checkFileUsage()
357369
case Filepointer::Operation::UNIMPORTANT:
358370
if (f.mode == OpenMode::CLOSED)
359371
useClosedFileError(tok);
372+
if (isftell && f.read_mode == Filepointer::ReadMode::READ_TEXT && printPortability)
373+
ftellFileError(tok);
360374
break;
361375
case Filepointer::Operation::UNKNOWN_OP:
362376
f.mode = OpenMode::UNKNOWN_OM;
@@ -424,6 +438,12 @@ void CheckIO::seekOnAppendedFileError(const Token *tok)
424438
"seekOnAppendedFile", "Repositioning operation performed on a file opened in append mode has no effect.", CWE398, Certainty::normal);
425439
}
426440

441+
void CheckIO::ftellFileError(const Token *tok)
442+
{
443+
reportError(tok, Severity::portability,
444+
"ftellTextModeFile", "ftell() result is unspecified when file is opened in mode \"t\"", CWE474, Certainty::normal);
445+
}
446+
427447
void CheckIO::incompatibleFileOpenError(const Token *tok, const std::string &filename)
428448
{
429449
reportError(tok, Severity::warning,

lib/checkio.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ class CPPCHECKLIB CheckIO : public Check {
107107
void useClosedFileError(const Token *tok);
108108
void fcloseInLoopConditionError(const Token *tok, const std::string &varname);
109109
void seekOnAppendedFileError(const Token *tok);
110+
void ftellFileError(const Token *tok);
110111
void incompatibleFileOpenError(const Token *tok, const std::string &filename);
111112
void invalidScanfError(const Token *tok);
112113
void wrongPrintfScanfArgumentsError(const Token* tok,
@@ -141,6 +142,7 @@ class CPPCHECKLIB CheckIO : public Check {
141142
"- Missing or wrong width specifiers in 'scanf' format string\n"
142143
"- Use a file that has been closed\n"
143144
"- File input/output without positioning results in undefined behaviour\n"
145+
"- Using 'ftell' on a file opened in text mode\n"
144146
"- Read to a file that has only been opened for writing (or vice versa)\n"
145147
"- Repositioning operation on a file opened in append mode\n"
146148
"- The same file can't be open for read and write at the same time on different streams\n"

man/checkers/ftellTextModeFile.md

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
# ftellModeTextFile
2+
3+
**Message**: ftell() result is unspecified when file is opened in mode "t".<br/>
4+
**Category**: Portability<br/>
5+
**Severity**: Style<br/>
6+
**Language**: C/C++
7+
8+
## Description
9+
10+
This checker detects the use of ftell() on a file open in text (or translate) mode. The text mode is not consistent
11+
in between Linux and Windows system and may cause ftell() to return the wrong offset inside a text file.
12+
13+
This warning helps improve code quality by:
14+
- Making the intent clear that the use of ftell() in "t" mode may cause portability problem.
15+
16+
## Motivation
17+
18+
This checker improves portability accross system.
19+
20+
## How to fix
21+
22+
According to C11, the file must be opened in binary mode 'b' to prevent this problem.
23+
24+
Before:
25+
```cpp
26+
FILE *f = fopen("Example.txt", "rt");
27+
if (f)
28+
{
29+
int position;
30+
struct stat st;
31+
32+
// Wrong way to get the file size
33+
fseek(f, 0, SEEK_END);
34+
printf( "File size %d\n, ftell(f));
35+
fclose(f);
36+
}
37+
38+
```
39+
40+
After:
41+
```cpp
42+
43+
FILE *f = fopen("Example.txt", "rb");
44+
if (f)
45+
{
46+
fseek(f, 0, SEEK_END);
47+
printf( "Offset %d\n", ftell(f);
48+
fclose(f);
49+
}
50+
51+
```
52+
53+
## Notes
54+
55+
See https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/fopen-wfopen?view=msvc-170
56+

releasenotes.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ New checks:
99
- funcArgNamesDifferentUnnamed warns on function declarations/definitions where a parameter in either location is unnamed
1010
- uninitMemberVarNoCtor warns on user-defined types where some but not all members requiring initialization have in-class initializers.
1111
- fcloseInLoopCondition warns when fclose() is used as a while loop condition, which may skip the loop body or double-close the file handle.
12+
- ftell() result is unspecified when file is opened in mode "t".
1213

1314
C/C++ support:
1415
-

test/testio.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ class TestIO : public TestFixture {
4444
TEST_CASE(fileIOwithoutPositioning);
4545
TEST_CASE(seekOnAppendedFile);
4646
TEST_CASE(fflushOnInputStream);
47+
TEST_CASE(ftellCompatibility);
4748
TEST_CASE(incompatibleFileOpen);
4849

4950
TEST_CASE(testScanf1); // Scanf without field limiters
@@ -727,6 +728,22 @@ class TestIO : public TestFixture {
727728
ASSERT_EQUALS("", errout_str()); // #6566
728729
}
729730

731+
void ftellCompatibility() {
732+
733+
check("void foo() {\n"
734+
" FILE *f = fopen(\"\", \"rt\");\n"
735+
" if (f)\n"
736+
" {\n"
737+
" extern long position;\n"
738+
" fseek(f, 0, SEEK_END);\n"
739+
" position = ftell(f);\n"
740+
" fclose(f);\n"
741+
" }\n"
742+
"}\n", dinit(CheckOptions, $.portability = true));
743+
ASSERT_EQUALS("[test.cpp:7:21]: (portability) ftell() result is unspecified when file is opened in mode \"t\" [ftellTextModeFile]\n", errout_str());
744+
}
745+
746+
730747
void fflushOnInputStream() {
731748
check("void foo()\n"
732749
"{\n"

0 commit comments

Comments
 (0)