Skip to content

Commit 7f6f075

Browse files
author
Tor Didriksen
committed
Bug#16078792 MY_WRITE() RETURNS WRONG VALUE ON DISK FULL
If my_write() hits a disk full condition, it can return a wrong value. Under certain conditions my_write() tries to repeat to write if it hits a disk full condition. Eventually it returns "writtenbytes+written" (if called without MY_NABP | MY_FNABP). 'writtenbytes' is the return value from the last write(2) call, and 'written' is the sum of bytes written successfully on earlier iterations. Now assume, my_write() was called to write 8192 bytes. After 4096 bytes it hits a disk full condition. That is, the first call to write(2) returns 4096. 'writtenbytes' is 4096. That value is accumulated to 'written'. In this case my_write() loops to read more data. But now it gets -1 from write(2) and errno is ENOSPC (for example). my_write() breaks the loop and returns "writtenbytes+written", which is 4095 now. (Even though both variables are unsigned, this is the outcome of the two's complement arithmetics). 4095 is not an appropriate return value in this situation. Since 4096 bytes have been written in fact, that value should be returned to the caller. The next call to my_write() should then return -1 if the disk is still full and no single byte could be written.
1 parent a2f4992 commit 7f6f075

File tree

3 files changed

+312
-34
lines changed

3 files changed

+312
-34
lines changed

‎mysys/my_write.c

+53-34
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,33 @@
1818
#include <errno.h>
1919

2020

21-
/* Write a chunk of bytes to a file */
21+
/**
22+
Write a chunk of bytes to a file
2223
24+
if (MyFlags & (MY_NABP | MY_FNABP))
25+
@returns
26+
0 if Count == 0
27+
On succes, 0
28+
On failure, (size_t)-1 == MY_FILE_ERROR
29+
30+
otherwise
31+
@returns
32+
0 if Count == 0
33+
On success, the number of bytes written.
34+
On partial success (if less than Count bytes could be written),
35+
the actual number of bytes written.
36+
On failure, (size_t)-1 == MY_FILE_ERROR
37+
*/
2338
size_t my_write(File Filedes, const uchar *Buffer, size_t Count, myf MyFlags)
2439
{
25-
size_t writtenbytes, written;
26-
uint errors;
40+
size_t writtenbytes;
41+
size_t sum_written= 0;
42+
uint errors= 0;
43+
const size_t initial_count= Count;
44+
2745
DBUG_ENTER("my_write");
2846
DBUG_PRINT("my",("fd: %d Buffer: %p Count: %lu MyFlags: %d",
2947
Filedes, Buffer, (ulong) Count, MyFlags));
30-
errors= 0; written= 0;
3148

3249
/* The behavior of write(fd, buf, 0) is not portable */
3350
if (unlikely(!Count))
@@ -37,6 +54,7 @@ size_t my_write(File Filedes, const uchar *Buffer, size_t Count, myf MyFlags)
3754
{ DBUG_SET("+d,simulate_file_write_error");});
3855
for (;;)
3956
{
57+
errno= 0;
4058
#ifdef _WIN32
4159
writtenbytes= my_win_write(Filedes, Buffer, Count);
4260
#else
@@ -48,10 +66,13 @@ size_t my_write(File Filedes, const uchar *Buffer, size_t Count, myf MyFlags)
4866
writtenbytes= (size_t) -1;
4967
});
5068
if (writtenbytes == Count)
69+
{
70+
sum_written+= writtenbytes;
5171
break;
72+
}
5273
if (writtenbytes != (size_t) -1)
5374
{ /* Safeguard */
54-
written+= writtenbytes;
75+
sum_written+= writtenbytes;
5576
Buffer+= writtenbytes;
5677
Count-= writtenbytes;
5778
}
@@ -72,39 +93,37 @@ size_t my_write(File Filedes, const uchar *Buffer, size_t Count, myf MyFlags)
7293
continue;
7394
}
7495

75-
if ((writtenbytes == 0 || writtenbytes == (size_t) -1))
96+
if (writtenbytes != 0 && writtenbytes != (size_t) -1)
97+
continue; /* Retry if something written */
98+
else if (my_errno == EINTR)
7699
{
77-
if (my_errno == EINTR)
78-
{
79-
DBUG_PRINT("debug", ("my_write() was interrupted and returned %ld",
80-
(long) writtenbytes));
81-
continue; /* Interrupted */
82-
}
83-
84-
if (!writtenbytes && !errors++) /* Retry once */
85-
{
86-
/* We may come here if the file quota is exeeded */
87-
errno= EFBIG; /* Assume this is the error */
88-
continue;
89-
}
100+
DBUG_PRINT("debug", ("my_write() was interrupted and returned %ld",
101+
(long) writtenbytes));
102+
continue; /* Interrupted, retry */
90103
}
91-
else
92-
continue; /* Retry */
93-
#endif
94-
if (MyFlags & (MY_NABP | MY_FNABP))
104+
else if (writtenbytes == 0 && !errors++) /* Retry once */
95105
{
96-
if (MyFlags & (MY_WME | MY_FAE | MY_FNABP))
97-
{
98-
char errbuf[MYSYS_STRERROR_SIZE];
99-
my_error(EE_WRITE, MYF(ME_BELL+ME_WAITTANG), my_filename(Filedes),
100-
my_errno, my_strerror(errbuf, sizeof(errbuf), my_errno));
101-
}
102-
DBUG_RETURN(MY_FILE_ERROR); /* Error on read */
106+
/* We may come here if the file quota is exeeded */
107+
continue;
103108
}
104-
else
105-
break; /* Return bytes written */
109+
#endif
110+
break;
106111
}
107112
if (MyFlags & (MY_NABP | MY_FNABP))
108-
DBUG_RETURN(0); /* Want only errors */
109-
DBUG_RETURN(writtenbytes+written);
113+
{
114+
if (sum_written == initial_count)
115+
DBUG_RETURN(0); /* Want only errors, not bytes written */
116+
if (MyFlags & (MY_WME | MY_FAE | MY_FNABP))
117+
{
118+
char errbuf[MYSYS_STRERROR_SIZE];
119+
my_error(EE_WRITE, MYF(ME_BELL+ME_WAITTANG), my_filename(Filedes),
120+
my_errno, my_strerror(errbuf, sizeof(errbuf), my_errno));
121+
}
122+
DBUG_RETURN(MY_FILE_ERROR);
123+
}
124+
125+
if (sum_written == 0)
126+
DBUG_RETURN(MY_FILE_ERROR);
127+
128+
DBUG_RETURN(sum_written);
110129
} /* my_write */

‎unittest/gunit/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,7 @@ SET(TESTS
239239
mysys_my_malloc
240240
mysys_my_rdtsc
241241
mysys_my_vsnprintf
242+
mysys_my_write
242243
sql_list
243244
sql_plist
244245
sql_string

‎unittest/gunit/mysys_my_write-t.cc

+258
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,258 @@
1+
/* Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
2+
3+
This program is free software; you can redistribute it and/or modify
4+
it under the terms of the GNU General Public License as published by
5+
the Free Software Foundation; version 2 of the License.
6+
7+
This program is distributed in the hope that it will be useful,
8+
but WITHOUT ANY WARRANTY; without even the implied warranty of
9+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
10+
GNU General Public License for more details.
11+
12+
You should have received a copy of the GNU General Public License
13+
along with this program; if not, write to the Free Software
14+
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02111-1307 USA */
15+
16+
// First include (the generated) my_config.h, to get correct platform defines.
17+
#include "my_config.h"
18+
#include <gtest/gtest.h>
19+
#include <gmock/gmock.h>
20+
21+
// Ignore test on windows, as we are mocking away a unix function, see below.
22+
#ifndef __WIN__
23+
namespace mysys_my_write_unittest {
24+
25+
using ::testing::_;
26+
using ::testing::InSequence;
27+
using ::testing::Return;
28+
using ::testing::SetErrnoAndReturn;
29+
30+
class MockWrite
31+
{
32+
public:
33+
virtual ~MockWrite() {}
34+
MOCK_METHOD3(mockwrite, ssize_t(int, const void *, size_t));
35+
};
36+
37+
MockWrite *mockfs= NULL;
38+
39+
// We need to mock away write(2), do it with a macro:
40+
#define write(fd, buf, count) mockfs->mockwrite(fd, buf, count)
41+
42+
/*
43+
Include the source file, which will give us
44+
mysys_my_write_unittest::my_write() for testing.
45+
*/
46+
#include "../../mysys/my_write.c"
47+
48+
#undef write
49+
50+
class MysysMyWriteTest : public ::testing::Test
51+
{
52+
virtual void SetUp()
53+
{
54+
mockfs= new MockWrite;
55+
}
56+
virtual void TearDown()
57+
{
58+
delete mockfs;
59+
mockfs= NULL;
60+
}
61+
};
62+
63+
64+
// Test of normal case: write OK
65+
TEST_F(MysysMyWriteTest, MyWriteOK)
66+
{
67+
uchar buf[4096];
68+
InSequence s;
69+
EXPECT_CALL(*mockfs, mockwrite(_, _, 4096))
70+
.Times(1)
71+
.WillOnce(Return(4096));
72+
73+
const size_t result= my_write(42, buf, 4096, 0);
74+
EXPECT_EQ(4096U, result);
75+
}
76+
77+
78+
// Test of normal case: write OK with MY_NABP
79+
TEST_F(MysysMyWriteTest, MyWriteOKNABP)
80+
{
81+
uchar buf[4096];
82+
InSequence s;
83+
EXPECT_CALL(*mockfs, mockwrite(_, _, 4096))
84+
.Times(1)
85+
.WillOnce(Return(4096));
86+
87+
const size_t result= my_write(42, buf, 4096, MYF(MY_NABP));
88+
EXPECT_EQ(0U, result);
89+
}
90+
91+
92+
// Test of disk full: write not OK
93+
TEST_F(MysysMyWriteTest, MyWriteFail)
94+
{
95+
uchar buf[4096];
96+
InSequence s;
97+
EXPECT_CALL(*mockfs, mockwrite(_, _, 4096))
98+
.Times(1)
99+
.WillOnce(SetErrnoAndReturn(ENOSPC, -1));
100+
101+
const size_t result= my_write(42, buf, 4096, 0);
102+
EXPECT_EQ(MY_FILE_ERROR, result);
103+
}
104+
105+
106+
// Test of disk full: write not OK, with MY_NABP
107+
TEST_F(MysysMyWriteTest, MyWriteFailNABP)
108+
{
109+
uchar buf[4096];
110+
InSequence s;
111+
EXPECT_CALL(*mockfs, mockwrite(_, _, 4096))
112+
.Times(1)
113+
.WillOnce(SetErrnoAndReturn(ENOSPC, -1));
114+
115+
const size_t result= my_write(42, buf, 4096, MYF(MY_NABP));
116+
EXPECT_EQ(MY_FILE_ERROR, result);
117+
}
118+
119+
120+
// Test of disk full after partial write.
121+
TEST_F(MysysMyWriteTest, MyWrite8192)
122+
{
123+
uchar buf[8192];
124+
InSequence s;
125+
// Expect call to write 8192 bytes, return 4096.
126+
EXPECT_CALL(*mockfs, mockwrite(_, _, 8192))
127+
.Times(1)
128+
.WillOnce(Return(4096));
129+
// Expect second call to write remaining 4096 bytes, return disk full.
130+
EXPECT_CALL(*mockfs, mockwrite(_, _, 4096))
131+
.Times(1)
132+
.WillOnce(SetErrnoAndReturn(ENOSPC, -1));
133+
134+
const size_t result= my_write(42, buf, 8192, 0);
135+
EXPECT_EQ(4096U, result);
136+
}
137+
138+
139+
// Test of disk full after partial write.
140+
TEST_F(MysysMyWriteTest, MyWrite8192NABP)
141+
{
142+
uchar buf[8192];
143+
InSequence s;
144+
// Expect call to write 8192 bytes, return 4096.
145+
EXPECT_CALL(*mockfs, mockwrite(_, _, 8192))
146+
.Times(1)
147+
.WillOnce(Return(4096));
148+
// Expect second call to write remaining 4096 bytes, return disk full.
149+
EXPECT_CALL(*mockfs, mockwrite(_, _, 4096))
150+
.Times(1)
151+
.WillOnce(SetErrnoAndReturn(ENOSPC, -1));
152+
153+
const size_t result= my_write(42, buf, 8192, MYF(MY_NABP));
154+
EXPECT_EQ(MY_FILE_ERROR, result);
155+
}
156+
157+
158+
// Test of partial write, followed by interrupt, followed by successful write.
159+
TEST_F(MysysMyWriteTest, MyWrite8192Interrupt)
160+
{
161+
uchar buf[8192];
162+
InSequence s;
163+
// Expect call to write 8192 bytes, return 4096.
164+
EXPECT_CALL(*mockfs, mockwrite(_, _, 8192))
165+
.Times(1)
166+
.WillOnce(Return(4096));
167+
// Expect second call to write remaining 4096 bytes, return interrupt.
168+
EXPECT_CALL(*mockfs, mockwrite(_, _, 4096))
169+
.Times(1)
170+
.WillOnce(SetErrnoAndReturn(EINTR, -1));
171+
// Expect third call to write remaining 4096 bytes, return 4096.
172+
EXPECT_CALL(*mockfs, mockwrite(_, _, 4096))
173+
.Times(1)
174+
.WillOnce(Return(4096));
175+
176+
const size_t result= my_write(42, buf, 8192, 0);
177+
EXPECT_EQ(8192U, result);
178+
}
179+
180+
181+
// Test of partial write, followed by interrupt, followed by successful write.
182+
TEST_F(MysysMyWriteTest, MyWrite8192InterruptNABP)
183+
{
184+
uchar buf[8192];
185+
InSequence s;
186+
// Expect call to write 8192 bytes, return 4096.
187+
EXPECT_CALL(*mockfs, mockwrite(_, _, 8192))
188+
.Times(1)
189+
.WillOnce(Return(4096));
190+
// Expect second call to write remaining 4096 bytes, return interrupt.
191+
EXPECT_CALL(*mockfs, mockwrite(_, _, 4096))
192+
.Times(1)
193+
.WillOnce(SetErrnoAndReturn(EINTR, -1));
194+
// Expect third call to write remaining 4096 bytes, return 4096.
195+
EXPECT_CALL(*mockfs, mockwrite(_, _, 4096))
196+
.Times(1)
197+
.WillOnce(Return(4096));
198+
199+
const size_t result= my_write(42, buf, 8192, MYF(MY_NABP));
200+
EXPECT_EQ(0U, result);
201+
}
202+
203+
204+
// Test of partial write, followed successful write.
205+
TEST_F(MysysMyWriteTest, MyWrite400)
206+
{
207+
uchar buf[400];
208+
InSequence s;
209+
EXPECT_CALL(*mockfs, mockwrite(_, _, 400))
210+
.Times(1)
211+
.WillOnce(Return(200));
212+
EXPECT_CALL(*mockfs, mockwrite(_, _, 200))
213+
.Times(1)
214+
.WillOnce(Return(200));
215+
216+
const size_t result= my_write(42, buf, 400, 0);
217+
EXPECT_EQ(400U, result);
218+
}
219+
220+
221+
// Test of partial write, followed successful write.
222+
TEST_F(MysysMyWriteTest, MyWrite400NABP)
223+
{
224+
uchar buf[400];
225+
InSequence s;
226+
EXPECT_CALL(*mockfs, mockwrite(_, _, 400))
227+
.Times(1)
228+
.WillOnce(Return(200));
229+
EXPECT_CALL(*mockfs, mockwrite(_, _, 200))
230+
.Times(1)
231+
.WillOnce(Return(200));
232+
233+
const size_t result= my_write(42, buf, 400, MYF(MY_NABP));
234+
EXPECT_EQ(0U, result);
235+
}
236+
237+
238+
// Test of partial write, followed by failure, followed successful write.
239+
TEST_F(MysysMyWriteTest, MyWrite300)
240+
{
241+
uchar buf[300];
242+
InSequence s;
243+
EXPECT_CALL(*mockfs, mockwrite(_, _, 300))
244+
.Times(1)
245+
.WillOnce(Return(100));
246+
EXPECT_CALL(*mockfs, mockwrite(_, _, 200))
247+
.Times(1)
248+
.WillOnce(SetErrnoAndReturn(EAGAIN, 0));
249+
EXPECT_CALL(*mockfs, mockwrite(_, _, 200))
250+
.Times(1)
251+
.WillOnce(Return(200));
252+
253+
const size_t result= my_write(42, buf, 300, 0);
254+
EXPECT_EQ(300U, result);
255+
}
256+
257+
}
258+
#endif

0 commit comments

Comments
 (0)