Drizzle Client & Protocol Library

Merge lp:~wiml-omni/libdrizzle/misc into lp:libdrizzle

  1. misc
  2. Merge into libdrizzle-redux
Proposed by Wim Lewis
Status: Merged
Approved by: Andrew Hutchings
Approved revision: 115
Merged at revision: 114
Proposed branch: lp:~wiml-omni/libdrizzle/misc
Merge into: lp:libdrizzle
Diff against target: 378 lines (+82/-89)
8 files modified
libdrizzle-5.1/statement.h (+1/-1)
libdrizzle/binlog.cc (+1/-0)
libdrizzle/common.h (+1/-0)
libdrizzle/conn.cc (+62/-84)
libdrizzle/conn_uds.cc (+0/-1)
libdrizzle/field.cc (+2/-0)
libdrizzle/statement_local.h (+1/-1)
libdrizzle/statement_param.cc (+14/-2)
To merge this branch: bzr merge lp:~wiml-omni/libdrizzle/misc
Reviewer Review Type Date Requested Status
Andrew Hutchings Approve
Review via email: mp+160736@code.launchpad.net

Description of the change

Miscellaneous improvements I have made while working with the library.

To post a comment you must log in.
Revision history for this message
Andrew Hutchings (linuxjedi) wrote :

"#if defined(EWOULDBLOCK) && (EWOULDBLOCK != EAGAIN)" could probably remove the "(EWOULDBLOCK != EAGAIN)" bit because you are removing the thing that makes them equal. But it should work fine anyway.

Thanks for the cleanups.

review: Approve
Revision history for this message
Wim Lewis (wiml-omni) wrote :

I was thinking of the case where the system header helpfully #defines EWOULDBLOCK to EAGAIN (or the reverse) --- (in fact, checking it just now, sys/errno.h on MacOSX does exactly that :) )

Revision history for this message
Andrew Hutchings (linuxjedi) wrote :

ah, cool. Nice catch then :)

Revision history for this message
Brian Aker (brianaker) wrote :
Download full text (12.0 KiB)

Why not use cinttypes?

It would be better to do this as the first include. inttypes is a bit annoying because whatever calls it determines its behavior.

On Apr 24, 2013, at 11:11 AM, Wim Lewis <email address hidden> wrote:

> Wim Lewis has proposed merging lp:~wiml-omni/libdrizzle/misc into lp:libdrizzle.
>
> Requested reviews:
> Drizzle Trunk (drizzle-trunk)
>
> For more details, see:
> https://code.launchpad.net/~wiml-omni/libdrizzle/misc/+merge/160736
>
> Miscellaneous improvements I have made while working with the library.
> --
> https://code.launchpad.net/~wiml-omni/libdrizzle/misc/+merge/160736
> Your team Drizzle Trunk is requested to review the proposed merge of lp:~wiml-omni/libdrizzle/misc into lp:libdrizzle.
> === modified file 'libdrizzle-5.1/statement.h'
> --- libdrizzle-5.1/statement.h 2013-01-27 11:55:48 +0000
> +++ libdrizzle-5.1/statement.h 2013-04-24 18:10:30 +0000
> @@ -96,7 +96,7 @@
> drizzle_return_t drizzle_stmt_set_float(drizzle_stmt_st *stmt, uint16_t param_num, float value);
>
> DRIZZLE_API
> -drizzle_return_t drizzle_stmt_set_string(drizzle_stmt_st *stmt, uint16_t param_num, char *value, size_t length);
> +drizzle_return_t drizzle_stmt_set_string(drizzle_stmt_st *stmt, uint16_t param_num, const char *value, size_t length);
>
> DRIZZLE_API
> drizzle_return_t drizzle_stmt_set_null(drizzle_stmt_st *stmt, uint16_t param_num);
>
> === modified file 'libdrizzle/binlog.cc'
> --- libdrizzle/binlog.cc 2013-03-06 18:09:39 +0000
> +++ libdrizzle/binlog.cc 2013-04-24 18:10:30 +0000
> @@ -39,6 +39,7 @@
> #include "libdrizzle/common.h"
>
> #include <zlib.h>
> +#include <inttypes.h>
>
> drizzle_binlog_st *drizzle_binlog_init(drizzle_st *con,
> drizzle_binlog_fn *binlog_fn,
>
> === modified file 'libdrizzle/common.h'
> --- libdrizzle/common.h 2013-01-27 11:55:48 +0000
> +++ libdrizzle/common.h 2013-04-24 18:10:30 +0000
> @@ -76,6 +76,7 @@
>
> #include <stddef.h>
> #include <stdarg.h>
> +#include <stdint.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
>
> === modified file 'libdrizzle/conn.cc'
> --- libdrizzle/conn.cc 2013-03-11 17:33:00 +0000
> +++ libdrizzle/conn.cc 2013-04-24 18:10:30 +0000
> @@ -45,34 +45,10 @@
> #include "config.h"
> #include "libdrizzle/common.h"
>
> -#ifndef SOCK_CLOEXEC
> -# define SOCK_CLOEXEC 0
> -#endif
> -
> -#ifndef SOCK_NONBLOCK
> -# define SOCK_NONBLOCK 0
> -#endif
> -
> -#ifndef FD_CLOEXEC
> -# define FD_CLOEXEC 0
> -#endif
> -
> -#ifndef F_GETFD
> -# define F_GETFD 0
> -#endif
> -
> -#ifndef MSG_DONTWAIT
> -# define MSG_DONTWAIT 0
> -#endif
> -
> #ifndef MSG_NOSIGNAL
> # define MSG_NOSIGNAL 0
> #endif
>
> -#ifndef EWOULDBLOCK
> -# define EWOULDBLOCK EAGAIN
> -#endif
> -
> #include <cerrno>
>
> /**
> @@ -1044,16 +1020,16 @@
>
> {
> int type= con->addrinfo_next->ai_socktype;
> - if (SOCK_CLOEXEC)
> - {
> - type|= SOCK_CLOEXEC;
> - }
> -
> - if (SOCK_NONBLOCK)
> - {
> - type|= SOCK_NONBLOCK;
> - }
> -
> +
> + /* Linuxisms to set some fd flags at the same time as creating the socket. */
> +#ifdef SOCK_CLOEXEC
> + type|= SOCK_CLOEXEC;
> +#endif
> +
...

Revision history for this message
Wim Lewis (wiml-omni) wrote :

You're right, <cinttypes> would be better here. (I'm mostly a C, not C++, programmer.)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libdrizzle-5.1/statement.h'
2--- libdrizzle-5.1/statement.h 2013-01-27 11:55:48 +0000
3+++ libdrizzle-5.1/statement.h 2013-04-24 18:10:30 +0000
4@@ -96,7 +96,7 @@
5 drizzle_return_t drizzle_stmt_set_float(drizzle_stmt_st *stmt, uint16_t param_num, float value);
6
7 DRIZZLE_API
8-drizzle_return_t drizzle_stmt_set_string(drizzle_stmt_st *stmt, uint16_t param_num, char *value, size_t length);
9+drizzle_return_t drizzle_stmt_set_string(drizzle_stmt_st *stmt, uint16_t param_num, const char *value, size_t length);
10
11 DRIZZLE_API
12 drizzle_return_t drizzle_stmt_set_null(drizzle_stmt_st *stmt, uint16_t param_num);
13
14=== modified file 'libdrizzle/binlog.cc'
15--- libdrizzle/binlog.cc 2013-03-06 18:09:39 +0000
16+++ libdrizzle/binlog.cc 2013-04-24 18:10:30 +0000
17@@ -39,6 +39,7 @@
18 #include "libdrizzle/common.h"
19
20 #include <zlib.h>
21+#include <inttypes.h>
22
23 drizzle_binlog_st *drizzle_binlog_init(drizzle_st *con,
24 drizzle_binlog_fn *binlog_fn,
25
26=== modified file 'libdrizzle/common.h'
27--- libdrizzle/common.h 2013-01-27 11:55:48 +0000
28+++ libdrizzle/common.h 2013-04-24 18:10:30 +0000
29@@ -76,6 +76,7 @@
30
31 #include <stddef.h>
32 #include <stdarg.h>
33+#include <stdint.h>
34 #include <stdio.h>
35 #include <stdlib.h>
36 #include <string.h>
37
38=== modified file 'libdrizzle/conn.cc'
39--- libdrizzle/conn.cc 2013-03-11 17:33:00 +0000
40+++ libdrizzle/conn.cc 2013-04-24 18:10:30 +0000
41@@ -45,34 +45,10 @@
42 #include "config.h"
43 #include "libdrizzle/common.h"
44
45-#ifndef SOCK_CLOEXEC
46-# define SOCK_CLOEXEC 0
47-#endif
48-
49-#ifndef SOCK_NONBLOCK
50-# define SOCK_NONBLOCK 0
51-#endif
52-
53-#ifndef FD_CLOEXEC
54-# define FD_CLOEXEC 0
55-#endif
56-
57-#ifndef F_GETFD
58-# define F_GETFD 0
59-#endif
60-
61-#ifndef MSG_DONTWAIT
62-# define MSG_DONTWAIT 0
63-#endif
64-
65 #ifndef MSG_NOSIGNAL
66 # define MSG_NOSIGNAL 0
67 #endif
68
69-#ifndef EWOULDBLOCK
70-# define EWOULDBLOCK EAGAIN
71-#endif
72-
73 #include <cerrno>
74
75 /**
76@@ -1044,16 +1020,16 @@
77
78 {
79 int type= con->addrinfo_next->ai_socktype;
80- if (SOCK_CLOEXEC)
81- {
82- type|= SOCK_CLOEXEC;
83- }
84-
85- if (SOCK_NONBLOCK)
86- {
87- type|= SOCK_NONBLOCK;
88- }
89-
90+
91+ /* Linuxisms to set some fd flags at the same time as creating the socket. */
92+#ifdef SOCK_CLOEXEC
93+ type|= SOCK_CLOEXEC;
94+#endif
95+
96+#ifdef SOCK_NONBLOCK
97+ type|= SOCK_NONBLOCK;
98+#endif
99+
100 con->fd= socket(con->addrinfo_next->ai_family,
101 type,
102 con->addrinfo_next->ai_protocol);
103@@ -1171,7 +1147,11 @@
104 }
105
106 ret= drizzle_set_events(con, POLLOUT);
107- if (con->options.non_blocking)
108+ if (ret != DRIZZLE_RETURN_OK)
109+ {
110+ return ret;
111+ }
112+ else if (con->options.non_blocking)
113 {
114 return DRIZZLE_RETURN_IO_WAIT;
115 }
116@@ -1220,6 +1200,8 @@
117
118 return DRIZZLE_RETURN_IO_WAIT;
119 }
120+
121+ assert(con->buffer_allocation > 0); /* Appease static analyzer */
122
123 while (1)
124 {
125@@ -1287,7 +1269,7 @@
126 switch (errno)
127 {
128 case EAGAIN:
129-#if EWOULDBLOCK != EAGAIN
130+#if defined(EWOULDBLOCK) && (EWOULDBLOCK != EAGAIN)
131 case EWOULDBLOCK:
132 #endif
133 {
134@@ -1463,32 +1445,25 @@
135 return DRIZZLE_RETURN_INVALID_ARGUMENT;
136 }
137
138-#ifdef HAVE_FCNTL
139- if (HAVE_FCNTL)
140+#if HAVE_FCNTL && !defined(SOCK_CLOEXEC) && defined(FD_CLOEXEC)
141 {
142- if (SOCK_CLOEXEC == 0)
143- {
144- if (FD_CLOEXEC and F_GETFD)
145+ int flags;
146+ do
147+ {
148+ flags= fcntl(con->fd, F_GETFD, 0);
149+ } while (flags == -1 and (errno == EINTR or errno == EAGAIN));
150+
151+ if (flags != -1)
152+ {
153+ int rval;
154+ do
155 {
156- int flags;
157- do
158- {
159- flags= fcntl(con->fd, F_GETFD, 0);
160- } while (flags == -1 and (errno == EINTR or errno == EAGAIN));
161-
162- if (flags != -1)
163- {
164- int rval;
165- do
166- {
167- rval= fcntl (con->fd, F_SETFD, flags | FD_CLOEXEC);
168- } while (rval == -1 && (errno == EINTR or errno == EAGAIN));
169- // we currently ignore the case where rval is -1
170- }
171- }
172+ rval= fcntl (con->fd, F_SETFD, flags | FD_CLOEXEC);
173+ } while (rval == -1 && (errno == EINTR or errno == EAGAIN));
174+ // we currently ignore the case where rval is -1
175 }
176 }
177-#endif // HAVE_FCNTL
178+#endif // HAVE_FCNTL but not SOCK_CLOEXEC
179
180
181 int ret= 1;
182@@ -1579,16 +1554,20 @@
183 }
184
185 #if defined(SO_NOSIGPIPE)
186- if (SO_NOSIGPIPE)
187- {
188- int ret= 1;
189- ret= setsockopt(con->fd, SOL_SOCKET, SO_NOSIGPIPE, static_cast<void *>(&ret), sizeof(int));
190-
191- if (ret == -1)
192- {
193- drizzle_set_error(con, __func__, "setsockopt(SO_NOSIGPIPE): %s", strerror(errno));
194- return DRIZZLE_RETURN_ERRNO;
195- }
196+ ret= setsockopt(con->fd, SOL_SOCKET, SO_NOSIGPIPE, static_cast<void *>(&ret), sizeof(int));
197+
198+ if (ret == -1)
199+ {
200+ drizzle_set_error(con, __func__, "setsockopt(SO_NOSIGPIPE): %s", strerror(errno));
201+ return DRIZZLE_RETURN_ERRNO;
202+ }
203+#elif HAVE_FCNTL && defined(F_SETNOSIGPIPE)
204+ ret= fcntl(con->fd, F_SETNOSIGPIPE, 1);
205+
206+ if (ret == -1)
207+ {
208+ drizzle_set_error(con, __func__, "fcntl:F_SETNOSIGPIPE:%s", strerror(errno));
209+ return DRIZZLE_RETURN_ERRNO;
210 }
211 #endif
212
213@@ -1602,22 +1581,21 @@
214 // @todo find out why this can't be non-blocking for SSL
215 if (!con->ssl)
216 {
217- if (SOCK_NONBLOCK == 0)
218- {
219- ret= fcntl(con->fd, F_GETFL, 0);
220- if (ret == -1)
221- {
222- drizzle_set_error(con, __func__, "fcntl:F_GETFL:%s", strerror(errno));
223- return DRIZZLE_RETURN_ERRNO;
224- }
225-
226- ret= fcntl(con->fd, F_SETFL, ret | O_NONBLOCK);
227- if (ret == -1)
228- {
229- drizzle_set_error(con, __func__, "fcntl:F_SETFL:%s", strerror(errno));
230- return DRIZZLE_RETURN_ERRNO;
231- }
232- }
233+#if HAVE_FCNTL && defined(O_NONBLOCK) && !defined(SOCK_NONBLOCK)
234+ ret= fcntl(con->fd, F_GETFL, 0);
235+ if (ret == -1)
236+ {
237+ drizzle_set_error(con, __func__, "fcntl:F_GETFL:%s", strerror(errno));
238+ return DRIZZLE_RETURN_ERRNO;
239+ }
240+
241+ ret= fcntl(con->fd, F_SETFL, ret | O_NONBLOCK);
242+ if (ret == -1)
243+ {
244+ drizzle_set_error(con, __func__, "fcntl:F_SETFL:%s", strerror(errno));
245+ return DRIZZLE_RETURN_ERRNO;
246+ }
247+#endif
248 }
249 #endif
250
251
252=== modified file 'libdrizzle/conn_uds.cc'
253--- libdrizzle/conn_uds.cc 2013-01-27 11:55:48 +0000
254+++ libdrizzle/conn_uds.cc 2013-04-24 18:10:30 +0000
255@@ -77,7 +77,6 @@
256
257 if (uds == NULL)
258 {
259- uds= "";
260 con->socket.uds.path_buffer[0]= 0;
261 }
262 else
263
264=== modified file 'libdrizzle/field.cc'
265--- libdrizzle/field.cc 2013-04-22 18:32:01 +0000
266+++ libdrizzle/field.cc 2013-04-24 18:10:30 +0000
267@@ -44,6 +44,8 @@
268 #include "config.h"
269 #include "libdrizzle/common.h"
270
271+#include <inttypes.h>
272+
273 /*
274 * Client definitions
275 */
276
277=== modified file 'libdrizzle/statement_local.h'
278--- libdrizzle/statement_local.h 2013-01-27 11:55:48 +0000
279+++ libdrizzle/statement_local.h 2013-04-24 18:10:30 +0000
280@@ -41,7 +41,7 @@
281 extern "C" {
282 #endif
283
284-drizzle_return_t drizzle_stmt_set_param(drizzle_stmt_st *stmt, uint16_t param_num, drizzle_column_type_t type, void *data, uint32_t length, bool is_unsigned);
285+drizzle_return_t drizzle_stmt_set_param(drizzle_stmt_st *stmt, uint16_t param_num, drizzle_column_type_t type, const void *data, size_t length, bool is_unsigned);
286
287 char *long_to_string(drizzle_bind_st *param, uint32_t val);
288
289
290=== modified file 'libdrizzle/statement_param.cc'
291--- libdrizzle/statement_param.cc 2013-04-23 20:36:28 +0000
292+++ libdrizzle/statement_param.cc 2013-04-24 18:10:30 +0000
293@@ -38,8 +38,12 @@
294 #include "config.h"
295 #include "libdrizzle/common.h"
296
297+#include <inttypes.h>
298+
299+#define CHECK_PARAM_NUM do { if (param_num > stmt->param_count) return DRIZZLE_RETURN_INVALID_ARGUMENT; } while(0)
300+
301 /* Internal function */
302-drizzle_return_t drizzle_stmt_set_param(drizzle_stmt_st *stmt, uint16_t param_num, drizzle_column_type_t type, void *data, uint32_t length, bool is_unsigned)
303+drizzle_return_t drizzle_stmt_set_param(drizzle_stmt_st *stmt, uint16_t param_num, drizzle_column_type_t type, const void *data, size_t length, bool is_unsigned)
304 {
305 if ((stmt == NULL) || (param_num >= stmt->param_count))
306 {
307@@ -64,6 +68,7 @@
308 drizzle_return_t drizzle_stmt_set_tiny(drizzle_stmt_st *stmt, uint16_t param_num, uint8_t value, bool is_unsigned)
309 {
310 uint8_t *val;
311+ CHECK_PARAM_NUM;
312 val= (uint8_t*) stmt->query_params[param_num].data_buffer;
313 *val= value;
314
315@@ -72,6 +77,7 @@
316 drizzle_return_t drizzle_stmt_set_short(drizzle_stmt_st *stmt, uint16_t param_num, uint16_t value, bool is_unsigned)
317 {
318 uint16_t *val;
319+ CHECK_PARAM_NUM;
320 val= (uint16_t*) stmt->query_params[param_num].data_buffer;
321 *val= value;
322
323@@ -81,6 +87,7 @@
324 drizzle_return_t drizzle_stmt_set_int(drizzle_stmt_st *stmt, uint16_t param_num, uint32_t value, bool is_unsigned)
325 {
326 uint32_t *val;
327+ CHECK_PARAM_NUM;
328 val= (uint32_t*) stmt->query_params[param_num].data_buffer;
329 *val= value;
330
331@@ -90,6 +97,7 @@
332 drizzle_return_t drizzle_stmt_set_bigint(drizzle_stmt_st *stmt, uint16_t param_num, uint64_t value, bool is_unsigned)
333 {
334 uint64_t *val;
335+ CHECK_PARAM_NUM;
336 val= (uint64_t*) stmt->query_params[param_num].data_buffer;
337 *val= value;
338
339@@ -99,6 +107,7 @@
340 drizzle_return_t drizzle_stmt_set_double(drizzle_stmt_st *stmt, uint16_t param_num, double value)
341 {
342 double *val;
343+ CHECK_PARAM_NUM;
344 val= (double*) stmt->query_params[param_num].data_buffer;
345 *val= value;
346
347@@ -108,13 +117,14 @@
348 drizzle_return_t drizzle_stmt_set_float(drizzle_stmt_st *stmt, uint16_t param_num, float value)
349 {
350 float *val;
351+ CHECK_PARAM_NUM;
352 val= (float*) stmt->query_params[param_num].data_buffer;
353 *val= value;
354
355 return drizzle_stmt_set_param(stmt, param_num, DRIZZLE_COLUMN_TYPE_FLOAT, val, 4, false);
356 }
357
358-drizzle_return_t drizzle_stmt_set_string(drizzle_stmt_st *stmt, uint16_t param_num, char *value, size_t length)
359+drizzle_return_t drizzle_stmt_set_string(drizzle_stmt_st *stmt, uint16_t param_num, const char *value, size_t length)
360 {
361 return drizzle_stmt_set_param(stmt, param_num, DRIZZLE_COLUMN_TYPE_STRING, value, length, false);
362 }
363@@ -127,6 +137,7 @@
364 drizzle_return_t drizzle_stmt_set_time(drizzle_stmt_st *stmt, uint16_t param_num, uint32_t days, uint8_t hours, uint8_t minutes, uint8_t seconds, uint32_t microseconds, bool is_negative)
365 {
366 drizzle_datetime_st *time;
367+ CHECK_PARAM_NUM;
368 time= (drizzle_datetime_st*) stmt->query_params[param_num].data_buffer;
369
370 memset(time, 0, sizeof(*time));
371@@ -145,6 +156,7 @@
372 drizzle_return_t drizzle_stmt_set_timestamp(drizzle_stmt_st *stmt, uint16_t param_num, uint16_t year, uint8_t month, uint8_t day, uint8_t hours, uint8_t minutes, uint8_t seconds, uint32_t microseconds)
373 {
374 drizzle_datetime_st *timestamp;
375+ CHECK_PARAM_NUM;
376 timestamp= (drizzle_datetime_st*) stmt->query_params[param_num].data_buffer;
377
378 memset(timestamp, 0, sizeof(*timestamp));

Subscribers

People subscribed via source and target branches

to all changes: