Merge lp:~brianaker/gearmand/gearman_connection_create-cleanup into lp:gearmand

Proposed by Brian Aker on 2013-06-25
Status: Merged
Merged at revision: 785
Proposed branch: lp:~brianaker/gearmand/gearman_connection_create-cleanup
Merge into: lp:gearmand
Diff against target: 533 lines (+91/-138)
10 files modified
libgearman/add.cc (+2/-2)
libgearman/backtrace.cc (+8/-9)
libgearman/client.cc (+2/-2)
libgearman/command.cc (+1/-1)
libgearman/connection.cc (+26/-65)
libgearman/connection.hpp (+16/-15)
libgearman/worker.cc (+1/-1)
tests/libgearman-1.0/internals.cc (+31/-31)
tests/libgearman-1.0/regression.cc (+2/-6)
tests/libgearman-1.0/worker_test.cc (+2/-6)
To merge this branch: bzr merge lp:~brianaker/gearmand/gearman_connection_create-cleanup
Reviewer Review Type Date Requested Status
Tangent Trunk 2013-06-25 Pending
Review via email: mp+171438@code.launchpad.net

Description of the change

Cleanup patch

To post a comment you must log in.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libgearman/add.cc'
2--- libgearman/add.cc 2013-06-11 06:48:11 +0000
3+++ libgearman/add.cc 2013-06-25 23:50:39 +0000
4@@ -286,8 +286,8 @@
5
6 case GEARMAN_COMMAND_SUBMIT_REDUCE_JOB:
7 case GEARMAN_COMMAND_SUBMIT_REDUCE_JOB_BACKGROUND:
8- assert(0);
9 rc= GEARMAN_INVALID_ARGUMENT;
10+ assert(rc != GEARMAN_INVALID_ARGUMENT);
11 break;
12
13 case GEARMAN_COMMAND_SUBMIT_JOB_SCHED:
14@@ -325,8 +325,8 @@
15 case GEARMAN_COMMAND_WORK_WARNING:
16 case GEARMAN_COMMAND_GET_STATUS_UNIQUE:
17 case GEARMAN_COMMAND_STATUS_RES_UNIQUE:
18- assert(0);
19 rc= GEARMAN_INVALID_ARGUMENT;
20+ assert(rc != GEARMAN_INVALID_ARGUMENT);
21 break;
22 }
23
24
25=== modified file 'libgearman/backtrace.cc'
26--- libgearman/backtrace.cc 2013-01-17 19:38:43 +0000
27+++ libgearman/backtrace.cc 2013-06-25 23:50:39 +0000
28@@ -51,13 +51,12 @@
29
30 #ifdef HAVE_GCC_ABI_DEMANGLE
31 # include <cxxabi.h>
32-# define USE_DEMANGLE 1
33-#else
34-# define USE_DEMANGLE 0
35-#endif
36-
37-#ifdef HAVE_DLFCN_H
38-# include <dlfcn.h>
39+# ifdef HAVE_DLFCN_H
40+# define USE_DEMANGLE 1
41+# include <dlfcn.h>
42+# else
43+# define USE_DEMANGLE 0
44+# endif
45 #endif
46
47 const int MAX_DEPTH= 50;
48@@ -78,9 +77,9 @@
49 {
50 bool was_demangled= false;
51
52+#if defined(USE_DEMANGLE)
53 if (USE_DEMANGLE)
54 {
55-#ifdef HAVE_DLFCN_H
56 Dl_info dlinfo;
57 if (dladdr(backtrace_buffer[x], &dlinfo))
58 {
59@@ -108,8 +107,8 @@
60 dlinfo.dli_fname);
61 }
62 }
63-#endif
64 }
65+#endif // USE_DEMANGLE
66
67 if (was_demangled == false)
68 {
69
70=== modified file 'libgearman/client.cc'
71--- libgearman/client.cc 2013-06-15 09:06:53 +0000
72+++ libgearman/client.cc 2013-06-25 23:50:39 +0000
73@@ -593,7 +593,7 @@
74 {
75 if (client_shell and client_shell->impl())
76 {
77- if (gearman_connection_create_args(client_shell->impl()->universal, host, port) == false)
78+ if (gearman_connection_create(client_shell->impl()->universal, host, port) == false)
79 {
80 assert(client_shell->impl()->universal.error_code() != GEARMAN_SUCCESS);
81 return client_shell->impl()->universal.error_code();
82@@ -608,7 +608,7 @@
83
84 gearman_return_t Client::add_server(const char *host, const char* service_)
85 {
86- if (gearman_connection_create_args(universal, host, service_) == false)
87+ if (gearman_connection_create(universal, host, service_) == false)
88 {
89 assert(universal.error_code() != GEARMAN_SUCCESS);
90 return universal.error_code();
91
92=== modified file 'libgearman/command.cc'
93--- libgearman/command.cc 2013-05-11 23:09:32 +0000
94+++ libgearman/command.cc 2013-06-25 23:50:39 +0000
95@@ -109,7 +109,7 @@
96 }
97
98 assert(0); // We should never reach this
99- abort();
100+ return "__UNKNOWN";
101 }
102
103 const gearman_command_info_st *gearman_command_info(gearman_command_t command)
104
105=== modified file 'libgearman/connection.cc'
106--- libgearman/connection.cc 2013-06-14 21:52:52 +0000
107+++ libgearman/connection.cc 2013-06-25 23:50:39 +0000
108@@ -79,35 +79,6 @@
109 # define MSG_DONTWAIT 0
110 #endif
111
112-static gearman_return_t gearman_connection_set_option(gearman_connection_st *connection,
113- gearman_connection_options_t options,
114- bool value)
115-{
116- switch (options)
117- {
118- case GEARMAN_CON_READY:
119- connection->options.ready= value;
120- break;
121-
122- case GEARMAN_CON_PACKET_IN_USE:
123- connection->options.packet_in_use= value;
124- break;
125-
126- case GEARMAN_CON_IGNORE_LOST_CONNECTION:
127- break;
128-
129- case GEARMAN_CON_CLOSE_AFTER_FLUSH:
130- break;
131-
132- case GEARMAN_CON_EXTERNAL_FD:
133- case GEARMAN_CON_MAX:
134- default:
135- return GEARMAN_INVALID_COMMAND;
136- }
137-
138- return GEARMAN_SUCCESS;
139-}
140-
141
142 /**
143 * @addtogroup gearman_connection_static Static Connection Declarations
144@@ -115,8 +86,7 @@
145 * @{
146 */
147
148-gearman_connection_st::gearman_connection_st(gearman_universal_st &universal_arg,
149- gearman_connection_options_t *options_args) :
150+gearman_connection_st::gearman_connection_st(gearman_universal_st &universal_arg) :
151 state(GEARMAN_CON_UNIVERSAL_ADDRINFO),
152 send_state(GEARMAN_CON_SEND_STATE_NONE),
153 recv_state(GEARMAN_CON_RECV_UNIVERSAL_NONE),
154@@ -142,15 +112,6 @@
155 send_buffer_ptr(NULL),
156 _recv_packet(NULL)
157 {
158- if (options_args)
159- {
160- while (*options_args != GEARMAN_CON_MAX)
161- {
162- gearman_connection_set_option(this, *options_args, true);
163- options_args++;
164- }
165- }
166-
167 if (universal.con_list)
168 {
169 universal.con_list->prev= this;
170@@ -165,10 +126,14 @@
171 _service[0]= 0;
172 }
173
174-gearman_connection_st *gearman_connection_create(gearman_universal_st &universal,
175- gearman_connection_options_t *options)
176+/**
177+ * Initialize a connection structure. Always check the return value even if
178+ * passing in a pre-allocated structure. Some other initialization may have
179+ * failed.
180+ */
181+static gearman_connection_st *__connection_create(gearman_universal_st &universal)
182 {
183- gearman_connection_st *connection= new (std::nothrow) gearman_connection_st(universal, options);
184+ gearman_connection_st *connection= new (std::nothrow) gearman_connection_st(universal);
185 if (connection == NULL)
186 {
187 gearman_perror(universal, "Failed at new() gearman_connection_st new");
188@@ -177,38 +142,34 @@
189 return connection;
190 }
191
192-gearman_connection_st *gearman_connection_create_args(gearman_universal_st& universal,
193- const char *host, const char* service_)
194+gearman_connection_st *gearman_connection_create(gearman_universal_st& universal,
195+ const char *host, const char* service_)
196 {
197- gearman_connection_st *connection= gearman_connection_create(universal);
198- if (connection == NULL)
199- {
200- return NULL;
201- }
202-
203- connection->set_host(host, service_);
204-
205- if (gearman_failed(connection->lookup()))
206- {
207- gearman_gerror(universal, GEARMAN_GETADDRINFO);
208- delete connection;
209- return NULL;
210+ gearman_connection_st *connection= __connection_create(universal);
211+ if (connection)
212+ {
213+ connection->set_host(host, (const char*)service_);
214+
215+ if (gearman_failed(connection->lookup()))
216+ {
217+ delete connection;
218+ return NULL;
219+ }
220 }
221
222 return connection;
223 }
224
225-gearman_connection_st *gearman_connection_create_args(gearman_universal_st& universal,
226- const char *host, in_port_t port)
227+gearman_connection_st *gearman_connection_create(gearman_universal_st& universal,
228+ const char *host, const in_port_t& port)
229 {
230- gearman_connection_st *connection= gearman_connection_create(universal, NULL);
231+ gearman_connection_st *connection= __connection_create(universal);
232 if (connection)
233 {
234 connection->set_host(host, port);
235
236 if (gearman_failed(connection->lookup()))
237 {
238- gearman_gerror(universal, GEARMAN_GETADDRINFO);
239 delete connection;
240 return NULL;
241 }
242@@ -220,7 +181,7 @@
243 gearman_connection_st *gearman_connection_copy(gearman_universal_st& universal,
244 const gearman_connection_st& from)
245 {
246- gearman_connection_st *connection= gearman_connection_create(universal);
247+ gearman_connection_st *connection= __connection_create(universal);
248
249 if (connection)
250 {
251@@ -278,7 +239,7 @@
252 * Public Definitions
253 */
254
255-void gearman_connection_st::set_host(const char *host_, const in_port_t& port_)
256+void gearman_connection_st::set_host(const char *host_, const in_port_t port_)
257 {
258 if (port_ < 1)
259 {
260@@ -683,7 +644,7 @@
261 if ((ret= getaddrinfo(_host, _service, &ai, &(_addrinfo))))
262 {
263 reset_addrinfo();
264- return gearman_universal_set_error(universal, GEARMAN_GETADDRINFO, GEARMAN_AT, "getaddrinfo:%s", gai_strerror(ret));
265+ return gearman_universal_set_error(universal, GEARMAN_GETADDRINFO, GEARMAN_AT, "%s:%d getaddrinfo:%s", host(), service(), gai_strerror(ret));
266 }
267
268 addrinfo_next= _addrinfo;
269
270=== modified file 'libgearman/connection.hpp'
271--- libgearman/connection.hpp 2013-06-11 01:05:38 +0000
272+++ libgearman/connection.hpp 2013-06-25 23:50:39 +0000
273@@ -115,14 +115,23 @@
274
275 void free_private_packet();
276
277- gearman_connection_st(gearman_universal_st &universal_arg,
278- gearman_connection_options_t *options);
279+ gearman_connection_st(gearman_universal_st &universal_arg);
280
281 ~gearman_connection_st();
282
283- void set_host( const char *host, const in_port_t& port);
284+ void set_host( const char *host, const in_port_t port);
285 void set_host( const char *host, const char* service);
286
287+ const char* host(void) const
288+ {
289+ return _host;
290+ }
291+
292+ const char* service(void) const
293+ {
294+ return _service;
295+ }
296+
297 gearman_return_t send_packet(const gearman_packet_st&, const bool flush_buffer);
298 size_t send_and_flush(const void *data, size_t data_size, gearman_return_t *ret_ptr);
299
300@@ -172,14 +181,6 @@
301 gearman_packet_st *_recv_packet;
302 };
303
304-/**
305- * Initialize a connection structure. Always check the return value even if
306- * passing in a pre-allocated structure. Some other initialization may have
307- * failed.
308- */
309-gearman_connection_st *gearman_connection_create(gearman_universal_st &universal,
310- gearman_connection_options_t *options= NULL);
311-
312 gearman_connection_st *gearman_connection_copy(gearman_universal_st& universal,
313 const gearman_connection_st& from);
314
315@@ -194,8 +195,8 @@
316 * @return On success, a pointer to the (possibly allocated) structure. On
317 * failure this will be NULL.
318 */
319-gearman_connection_st *gearman_connection_create_args(gearman_universal_st &universal,
320- const char *host, in_port_t port);
321+gearman_connection_st *gearman_connection_create(gearman_universal_st &universal,
322+ const char *host, const in_port_t&);
323
324-gearman_connection_st *gearman_connection_create_args(gearman_universal_st &universal,
325- const char* host, const char* service);
326+gearman_connection_st *gearman_connection_create(gearman_universal_st &universal,
327+ const char* host, const char* service);
328
329=== modified file 'libgearman/worker.cc'
330--- libgearman/worker.cc 2013-06-11 01:05:38 +0000
331+++ libgearman/worker.cc 2013-06-25 23:50:39 +0000
332@@ -445,7 +445,7 @@
333 {
334 if (worker and worker->impl())
335 {
336- if (gearman_connection_create_args(worker->impl()->universal, host, port) == NULL)
337+ if (gearman_connection_create(worker->impl()->universal, host, port) == NULL)
338 {
339 return gearman_universal_error_code(worker->impl()->universal);
340 }
341
342=== modified file 'tests/libgearman-1.0/internals.cc'
343--- tests/libgearman-1.0/internals.cc 2013-06-11 06:48:11 +0000
344+++ tests/libgearman-1.0/internals.cc 2013-06-25 23:50:39 +0000
345@@ -94,23 +94,23 @@
346 gearman_universal_clone(destination, gear);
347
348 { // Test all of the flags
349- test_truth(destination.options.non_blocking == gear.options.non_blocking);
350+ ASSERT_EQ(destination.options.non_blocking, gear.options.non_blocking);
351 }
352- test_truth(destination._namespace == gear._namespace);
353- test_truth(destination.verbose == gear.verbose);
354- test_truth(destination.con_count == gear.con_count);
355- test_truth(destination.packet_count == gear.packet_count);
356- test_truth(destination.pfds_size == gear.pfds_size);
357- test_truth(destination._error.last_errno == gear._error.last_errno);
358- test_truth(destination.timeout == gear.timeout);
359- test_truth(destination.con_list == gear.con_list);
360- test_truth(destination.packet_list == gear.packet_list);
361- test_truth(destination.pfds == gear.pfds);
362- test_truth(destination.log_fn == gear.log_fn);
363- test_truth(destination.log_context == gear.log_context);
364- test_truth(destination.allocator.malloc == gear.allocator.malloc);
365- test_truth(destination.allocator.context == gear.allocator.context);
366- test_truth(destination.allocator.free == gear.allocator.free);
367+ ASSERT_EQ(destination._namespace, gear._namespace);
368+ ASSERT_EQ(destination.verbose, gear.verbose);
369+ ASSERT_EQ(destination.con_count, gear.con_count);
370+ ASSERT_EQ(destination.packet_count, gear.packet_count);
371+ ASSERT_EQ(destination.pfds_size, gear.pfds_size);
372+ ASSERT_EQ(destination._error.last_errno, gear._error.last_errno);
373+ ASSERT_EQ(destination.timeout, gear.timeout);
374+ ASSERT_EQ(destination.con_list, gear.con_list);
375+ ASSERT_EQ(destination.packet_list, gear.packet_list);
376+ ASSERT_EQ(destination.pfds, gear.pfds);
377+ ASSERT_EQ(destination.log_fn, gear.log_fn);
378+ ASSERT_EQ(destination.log_context, gear.log_context);
379+ ASSERT_EQ(destination.allocator.malloc, gear.allocator.malloc);
380+ ASSERT_EQ(destination.allocator.context, gear.allocator.context);
381+ ASSERT_EQ(destination.allocator.free, gear.allocator.free);
382
383 gearman_universal_free(gear);
384 }
385@@ -171,7 +171,7 @@
386 gearman_universal_st universal(options);
387
388 { // Initial Allocated, no changes
389- test_truth(universal.options.non_blocking);
390+ ASSERT_TRUE(universal.options.non_blocking);
391 }
392 gearman_universal_free(universal);
393
394@@ -200,11 +200,11 @@
395 ASSERT_FALSE(universal._namespace);
396
397 gearman_universal_set_namespace(universal, gearman_literal_param("my_dog"));
398- test_truth(universal._namespace);
399+ ASSERT_TRUE(universal._namespace);
400
401 gearman_universal_st clone;
402 gearman_universal_clone(clone, universal);
403- test_truth(clone._namespace);
404+ ASSERT_TRUE(clone._namespace);
405
406 gearman_universal_free(universal);
407 gearman_universal_free(clone);
408@@ -218,7 +218,7 @@
409 gearman_universal_st universal(options);
410
411 { // Initial Allocated, no changes
412- test_truth(universal.options.non_blocking);
413+ ASSERT_TRUE(universal.options.non_blocking);
414 }
415
416 ASSERT_TRUE(gearman_universal_is_non_blocking(universal));
417@@ -259,8 +259,8 @@
418 {
419 gearman_universal_st universal;
420
421- gearman_connection_st *connection_ptr= gearman_connection_create(universal, NULL);
422- test_truth(connection_ptr);
423+ gearman_connection_st *connection_ptr= gearman_connection_create(universal, NULL, (const char*)(GEARMAN_DEFAULT_TCP_PORT_STRING));
424+ ASSERT_TRUE(connection_ptr);
425
426 ASSERT_FALSE(connection_ptr->options.ready);
427 ASSERT_FALSE(connection_ptr->options.packet_in_use);
428@@ -274,8 +274,8 @@
429 {
430 gearman_universal_st universal;
431
432- gearman_connection_st *connection_ptr= gearman_connection_create(universal, NULL);
433- test_truth(connection_ptr);
434+ gearman_connection_st *connection_ptr= gearman_connection_create(universal, NULL, (const char*)(GEARMAN_DEFAULT_TCP_PORT_STRING));
435+ ASSERT_TRUE(connection_ptr);
436
437 ASSERT_FALSE(connection_ptr->options.ready);
438 ASSERT_FALSE(connection_ptr->options.packet_in_use);
439@@ -305,7 +305,7 @@
440 ASSERT_FALSE(packet.options.complete);
441 ASSERT_FALSE(packet.options.free_data);
442
443- test_truth(packet_ptr == &packet);
444+ ASSERT_EQ(packet_ptr, &packet);
445
446 gearman_packet_free(packet_ptr);
447 ASSERT_FALSE(packet.options.is_allocated);
448@@ -326,13 +326,13 @@
449
450 gearman_packet_st packet;
451
452- test_truth(gearman_packet_create(universal, packet));
453+ ASSERT_TRUE(gearman_packet_create(universal, packet));
454
455 gearman_packet_give_data(packet, data, data_size);
456
457- test_truth(packet.data == data);
458+ ASSERT_EQ(packet.data, data);
459 ASSERT_EQ(packet.data_size, data_size);
460- test_truth(packet.options.free_data);
461+ ASSERT_TRUE(packet.options.free_data);
462
463 gearman_packet_free(&packet);
464 gearman_universal_free(universal);
465@@ -354,13 +354,13 @@
466 gearman_packet_st packet;
467
468 gearman_packet_st *packet_ptr= gearman_packet_create(universal, packet);
469- test_truth(packet_ptr);
470+ ASSERT_TRUE(packet_ptr);
471
472 gearman_packet_give_data(packet, data, data_size);
473
474- test_truth(packet_ptr->data == data);
475+ ASSERT_EQ(packet_ptr->data, data);
476 ASSERT_EQ(data_size, packet_ptr->data_size);
477- test_truth(packet_ptr->options.free_data);
478+ ASSERT_TRUE(packet_ptr->options.free_data);
479
480 size_t mine_size;
481 char *mine= (char *)gearman_packet_take_data(packet, &mine_size);
482
483=== modified file 'tests/libgearman-1.0/regression.cc'
484--- tests/libgearman-1.0/regression.cc 2013-01-03 02:58:19 +0000
485+++ tests/libgearman-1.0/regression.cc 2013-06-25 23:50:39 +0000
486@@ -126,9 +126,7 @@
487 {
488 gearman_packet_st packet;
489 gearman_connection_st *con_ptr;
490- test_truth(con_ptr= gearman_connection_create(universal, NULL));
491-
492- con_ptr->set_host(NULL, default_port());
493+ test_truth(con_ptr= gearman_connection_create(universal, NULL, default_port()));
494
495 args[0]= "testUnregisterFunction";
496 args_size[0]= strlen("testUnregisterFunction");
497@@ -159,9 +157,7 @@
498
499 delete con_ptr;
500
501- test_truth(con_ptr= gearman_connection_create(universal, NULL));
502-
503- con_ptr->set_host(NULL, default_port());
504+ test_truth(con_ptr= gearman_connection_create(universal, NULL, default_port()));
505
506 args[0]= "testUnregisterFunction";
507 args_size[0]= strlen("testUnregisterFunction");
508
509=== modified file 'tests/libgearman-1.0/worker_test.cc'
510--- tests/libgearman-1.0/worker_test.cc 2013-06-15 01:53:12 +0000
511+++ tests/libgearman-1.0/worker_test.cc 2013-06-25 23:50:39 +0000
512@@ -888,9 +888,7 @@
513 gearman_universal_st universal;
514
515 gearman_connection_st *connection1;
516- test_truth(connection1= gearman_connection_create(universal, NULL));
517-
518- connection1->set_host(NULL, libtest::default_port());
519+ test_truth(connection1= gearman_connection_create(universal, NULL, default_port()));
520
521 gearman_packet_st packet;
522 args[0]= "abandoned_worker";
523@@ -921,9 +919,7 @@
524 gearman_packet_free(&packet);
525
526 gearman_connection_st *connection2;
527- test_truth(connection2= gearman_connection_create(universal, NULL));
528-
529- connection2->set_host(NULL, libtest::default_port());
530+ test_truth(connection2= gearman_connection_create(universal, NULL, default_port()));
531
532 args[0]= "abandoned_worker";
533 args_size[0]= strlen("abandoned_worker");

Subscribers

People subscribed via source and target branches

to all changes: