Merge lp:~s-eilting/widelands/bug-1203436 into lp:widelands
- bug-1203436
- Merge into trunk
Status: | Merged | ||||
---|---|---|---|---|---|
Merged at revision: | 7379 | ||||
Proposed branch: | lp:~s-eilting/widelands/bug-1203436 | ||||
Merge into: | lp:widelands | ||||
Diff against target: |
678 lines (+183/-138) 12 files modified
cmake/codecheck/rules/do_not_use_strcasecmp (+18/-0) cmake/codecheck/rules/do_not_use_strdup (+21/-0) src/ai/ai_hints.cc (+7/-16) src/ai/ai_hints.h (+15/-7) src/ai/defaultai.cc (+4/-4) src/editor/ui_menus/editor_main_menu_save_map.cc (+2/-9) src/logic/building.cc (+10/-7) src/logic/immovable.cc (+9/-5) src/logic/save_handler.cc (+2/-9) src/map_io/widelands_map_loader.h (+2/-2) src/profile/profile.cc (+69/-64) src/profile/profile.h (+24/-15) |
||||
To merge this branch: | bzr merge lp:~s-eilting/widelands/bug-1203436 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
SirVer | Approve | ||
Tino | Pending | ||
Review via email: mp+247972@code.launchpad.net |
This proposal supersedes a proposal from 2015-01-27.
Commit message
Description of the change
Mostly, remove strdup, strcasecmp and related stuff (see bug 1203436, https:/
SirVer (sirver) wrote : Posted in a previous version of this proposal | # |
Tino (tino79) wrote : Posted in a previous version of this proposal | # |
Does compile fine on windows with mingw32-w64.
Can I help with the putenv() you discussed in the bug report?
I can only find 2 occurences (and one is _putenv) in the codebase, both in a #ifdef _win32 block.
Simon Eilting (s-eilting) wrote : Posted in a previous version of this proposal | # |
> Can I help with the putenv() you discussed in the bug report?
> I can only find 2 occurences (and one is _putenv) in the codebase, both in a
> #ifdef _win32 block.
According to docs, calls to putenv(
Simon Eilting (s-eilting) wrote : | # |
Have updated the branch with sirver's minor nits fixed.
SirVer (sirver) wrote : | # |
Great. No more issues from my side. Two more questions though:
1) Do you guys want to figure out putenv in this branch? Otherwise I can merge this and a separate branch can deal with that.
2) Simon, can we expect more contributions from you. If so, have you something specific in mind or would you like a selection of important/fun topics to choose from?
Ps: Whoever merges this, please remember to add simon to txt/developers. I keep forgetting this with new contributors.
Simon Eilting (s-eilting) wrote : | # |
Sirver:
2) Seems like a fun project, and I would like to help out. Have certainly played enough to guilt-trip myself into it. ;) Got any suggestions?
Simon Eilting (s-eilting) wrote : | # |
Tino:
Are you waiting for input from me? Tell me what you expect.
Simon Eilting (s-eilting) wrote : | # |
(Oh, I see. Never mind.)
SirVer (sirver) wrote : | # |
Simon, we have tons of stuff. It really depends on what you enjoy and what you want to work on. I threw a bunch of quick ideas into this document, most of them need fleshing out a bit before implementation can start, but feel free to pick any:
https:/
GunChleoc (gunchleoc) wrote : | # |
I have added the font handler to the list - I could really use some help there.
Simon Eilting (s-eilting) wrote : | # |
Mostly at random, I think I'd like to try helping with the One Tribe bug.
As you've noticed by now, I don't always have enough free time, but generally it should be enough to make a difference.
SirVer (sirver) wrote : | # |
Great, I fleshed out the project a bit and moved it to a separate design doc:
https:/
Gun knows most about the current state of affairs, but I think the current implementation is somewhere around step 2 - 4. Gun, could you prove read the document if it all makes sense to you?
Simon, I added you to the widelands-dev, so that you can push and pull to the branch and collaboration can happen. If you have any questions, the team list is <email address hidden> (you are subscribed as team member) and the community including the devs hang out in #widelands on freenode.
Please dig in!
Preview Diff
1 | === added file 'cmake/codecheck/rules/do_not_use_strcasecmp' |
2 | --- cmake/codecheck/rules/do_not_use_strcasecmp 1970-01-01 00:00:00 +0000 |
3 | +++ cmake/codecheck/rules/do_not_use_strcasecmp 2015-01-29 12:29:13 +0000 |
4 | @@ -0,0 +1,18 @@ |
5 | +#!/usr/bin/python |
6 | + |
7 | +""" |
8 | +strcasecmp isn't available on win32 |
9 | +""" |
10 | + |
11 | +error_msg = 'Do not use strcasecmp/strncasecmp. Use boost::iequals instead.' |
12 | + |
13 | +regexp = r'\bstrn?casecmp\s*\(' |
14 | + |
15 | +allowed = [ |
16 | + 'boost::iequals(a, b)', |
17 | +] |
18 | + |
19 | +forbidden = [ |
20 | + 'strcasecmp(a, b)', |
21 | + '!strncasecmp(a, b, l)', |
22 | +] |
23 | |
24 | === added file 'cmake/codecheck/rules/do_not_use_strdup' |
25 | --- cmake/codecheck/rules/do_not_use_strdup 1970-01-01 00:00:00 +0000 |
26 | +++ cmake/codecheck/rules/do_not_use_strdup 2015-01-29 12:29:13 +0000 |
27 | @@ -0,0 +1,21 @@ |
28 | +#!/usr/bin/python |
29 | + |
30 | +""" |
31 | +strdup isn't available on win32 |
32 | +""" |
33 | + |
34 | +error_msg = 'Do not use strdup/strndup. Use std::string instead.' |
35 | + |
36 | +regexp = r'\bstrn?dup\s*\(' |
37 | + |
38 | +allowed = [ |
39 | + 'y = std::string(x)', |
40 | + |
41 | + 'y = new char[strlen(x)+1]', |
42 | + 'std::copy(x, x+strlen(x)+1, y)', |
43 | +] |
44 | + |
45 | +forbidden = [ |
46 | + 'y = strdup(x)', |
47 | + 'y = strndup(x)', |
48 | +] |
49 | |
50 | === modified file 'src/ai/ai_hints.cc' |
51 | --- src/ai/ai_hints.cc 2014-07-22 20:18:11 +0000 |
52 | +++ src/ai/ai_hints.cc 2015-01-29 12:29:13 +0000 |
53 | @@ -19,20 +19,10 @@ |
54 | |
55 | #include "ai/ai_hints.h" |
56 | |
57 | -#include <cstdlib> |
58 | -#include <cstring> |
59 | - |
60 | #include "profile/profile.h" |
61 | |
62 | -BuildingHints::~BuildingHints() { |
63 | - free(renews_map_resource); |
64 | - free(mines_); |
65 | -} |
66 | - |
67 | BuildingHints::BuildingHints(Section* const section) |
68 | - : renews_map_resource(nullptr), |
69 | - mines_(nullptr), |
70 | - log_producer_(section ? section->get_bool("logproducer") : false), |
71 | + : log_producer_(section ? section->get_bool("logproducer") : false), |
72 | stone_producer_(section ? section->get_bool("stoneproducer") : false), |
73 | needs_water_(section ? section->get_bool("needs_water") : false), |
74 | mines_water_(section ? section->get_bool("mines_water") : false), |
75 | @@ -43,11 +33,12 @@ |
76 | mountain_conqueror_(section ? section->get_bool("mountain_conqueror") : false), |
77 | prohibited_till_(section ? section->get_int("prohibited_till", 0) : 0), |
78 | forced_after_(section ? section->get_int("forced_after", 864000) : 0), // 10 days default |
79 | - mines_percent_(section ? section->get_int("mines_percent", 100) : 0) { |
80 | + mines_percent_(section ? section->get_int("mines_percent", 100) : 0) |
81 | +{ |
82 | if (section) { |
83 | - if (char const* const s = section->get_string("renews_map_resource")) |
84 | - renews_map_resource = strdup(s); |
85 | - if (char const* const s = section->get_string("mines")) |
86 | - mines_ = strdup(s); |
87 | + if (section->has_val("renews_map_resource")) |
88 | + renews_map_resource_ = section->get_string("renews_map_resource"); |
89 | + if (section->has_val("mines")) |
90 | + mines_ = section->get_string("mines"); |
91 | } |
92 | } |
93 | |
94 | === modified file 'src/ai/ai_hints.h' |
95 | --- src/ai/ai_hints.h 2014-07-22 20:18:11 +0000 |
96 | +++ src/ai/ai_hints.h 2015-01-29 12:29:13 +0000 |
97 | @@ -21,6 +21,7 @@ |
98 | #define WL_AI_AI_HINTS_H |
99 | |
100 | #include <stdint.h> |
101 | +#include <string> |
102 | |
103 | #include "base/macros.h" |
104 | |
105 | @@ -31,14 +32,21 @@ |
106 | /// special properties of a building. |
107 | struct BuildingHints { |
108 | BuildingHints(Section*); |
109 | - ~BuildingHints(); |
110 | - |
111 | - char const* get_renews_map_resource() const { |
112 | - return renews_map_resource; |
113 | + |
114 | + bool renews_map_resource() const { |
115 | + return !renews_map_resource_.empty(); |
116 | + } |
117 | + |
118 | + std::string get_renews_map_resource() const { |
119 | + return renews_map_resource_; |
120 | + } |
121 | + |
122 | + bool has_mines() const { |
123 | + return !mines_.empty(); |
124 | } |
125 | |
126 | char const* get_mines() const { |
127 | - return mines_; |
128 | + return mines_.c_str(); |
129 | } |
130 | |
131 | bool is_logproducer() const { |
132 | @@ -87,8 +95,8 @@ |
133 | } |
134 | |
135 | private: |
136 | - char* renews_map_resource; |
137 | - char* mines_; |
138 | + std::string renews_map_resource_; |
139 | + std::string mines_; |
140 | bool log_producer_; |
141 | bool stone_producer_; |
142 | bool needs_water_; |
143 | |
144 | === modified file 'src/ai/defaultai.cc' |
145 | --- src/ai/defaultai.cc 2015-01-24 11:30:20 +0000 |
146 | +++ src/ai/defaultai.cc 2015-01-29 12:29:13 +0000 |
147 | @@ -320,8 +320,8 @@ |
148 | bo.mountain_conqueror_ = bh.is_mountain_conqueror(); |
149 | bo.prohibited_till_ = bh.get_prohibited_till() * 1000; // value in conf is in seconds |
150 | bo.forced_after_ = bh.get_forced_after() * 1000; // value in conf is in seconds |
151 | - if (char const* const s = bh.get_renews_map_resource()) { |
152 | - bo.production_hint_ = tribe_->safe_ware_index(s); |
153 | + if (bh.renews_map_resource()) { |
154 | + bo.production_hint_ = tribe_->safe_ware_index(bh.get_renews_map_resource()); |
155 | } |
156 | |
157 | // I just presume cut wood is named "log" in the game |
158 | @@ -345,8 +345,8 @@ |
159 | |
160 | if (bo.type == BuildingObserver::MINE) { |
161 | // get the resource needed by the mine |
162 | - if (char const* const s = bh.get_mines()) { |
163 | - bo.mines_ = world.get_resource(s); |
164 | + if (bh.has_mines()) { |
165 | + bo.mines_ = world.get_resource(bh.get_mines()); |
166 | } |
167 | |
168 | bo.mines_percent_ = bh.get_mines_percent(); |
169 | |
170 | === modified file 'src/editor/ui_menus/editor_main_menu_save_map.cc' |
171 | --- src/editor/ui_menus/editor_main_menu_save_map.cc 2014-11-30 18:49:38 +0000 |
172 | +++ src/editor/ui_menus/editor_main_menu_save_map.cc 2015-01-29 12:29:13 +0000 |
173 | @@ -24,6 +24,7 @@ |
174 | #include <memory> |
175 | #include <string> |
176 | |
177 | +#include <boost/algorithm/string.hpp> |
178 | #include <boost/format.hpp> |
179 | |
180 | #include "base/i18n.h" |
181 | @@ -358,15 +359,7 @@ |
182 | g_fs->ensure_directory_exists(m_basedir); |
183 | |
184 | // OK, first check if the extension matches (ignoring case). |
185 | - bool assign_extension = true; |
186 | - if (filename.size() >= strlen(WLMF_SUFFIX)) { |
187 | - char buffer[10]; // enough for the extension |
188 | - filename.copy |
189 | - (buffer, sizeof(WLMF_SUFFIX), filename.size() - strlen(WLMF_SUFFIX)); |
190 | - if (!strncasecmp(buffer, WLMF_SUFFIX, strlen(WLMF_SUFFIX))) |
191 | - assign_extension = false; |
192 | - } |
193 | - if (assign_extension) |
194 | + if (!boost::iends_with(filename, WLMF_SUFFIX)) |
195 | filename += WLMF_SUFFIX; |
196 | |
197 | // append directory name |
198 | |
199 | === modified file 'src/logic/building.cc' |
200 | --- src/logic/building.cc 2014-12-28 16:45:37 +0000 |
201 | +++ src/logic/building.cc 2015-01-29 12:29:13 +0000 |
202 | @@ -22,6 +22,7 @@ |
203 | #include <cstdio> |
204 | #include <sstream> |
205 | |
206 | +#include <boost/algorithm/string.hpp> |
207 | #include <boost/format.hpp> |
208 | |
209 | #include "base/macros.h" |
210 | @@ -69,24 +70,26 @@ |
211 | m_global (false), |
212 | m_vision_range (0) |
213 | { |
214 | + using boost::iequals; |
215 | + |
216 | try { |
217 | - char const * const string = global_s.get_safe_string("size"); |
218 | - if (!strcasecmp(string, "small")) |
219 | + const auto& size = global_s.get_safe_string("size"); |
220 | + if (iequals(size, "small")) |
221 | m_size = BaseImmovable::SMALL; |
222 | - else if (!strcasecmp(string, "medium")) |
223 | + else if (iequals(size, "medium")) |
224 | m_size = BaseImmovable::MEDIUM; |
225 | - else if (!strcasecmp(string, "big")) |
226 | + else if (iequals(size, "big")) |
227 | m_size = BaseImmovable::BIG; |
228 | - else if (!strcasecmp(string, "mine")) { |
229 | + else if (iequals(size, "mine")) { |
230 | m_size = BaseImmovable::SMALL; |
231 | m_mine = true; |
232 | - } else if (!strcasecmp(string, "port")) { |
233 | + } else if (iequals(size, "port")) { |
234 | m_size = BaseImmovable::BIG; |
235 | m_port = true; |
236 | } else |
237 | throw GameDataError |
238 | ("expected %s but found \"%s\"", |
239 | - "{\"small\"|\"medium\"|\"big\"|\"port\"|\"mine\"}", string); |
240 | + "{\"small\"|\"medium\"|\"big\"|\"port\"|\"mine\"}", size); |
241 | } catch (const WException & e) { |
242 | throw GameDataError("size: %s", e.what()); |
243 | } |
244 | |
245 | === modified file 'src/logic/immovable.cc' |
246 | --- src/logic/immovable.cc 2014-12-06 12:22:35 +0000 |
247 | +++ src/logic/immovable.cc 2015-01-29 12:29:13 +0000 |
248 | @@ -230,21 +230,25 @@ |
249 | m_size (BaseImmovable::NONE), |
250 | m_owner_tribe (owner_tribe) |
251 | { |
252 | - if (char const * const string = global_s.get_string("size")) |
253 | + using boost::iequals; |
254 | + |
255 | + if (global_s.has_val("size")) { |
256 | + const auto& size = global_s.get_string("size"); |
257 | try { |
258 | - if (!strcasecmp(string, "small")) |
259 | + if (iequals(size, "small")) |
260 | m_size = BaseImmovable::SMALL; |
261 | - else if (!strcasecmp(string, "medium")) |
262 | + else if (iequals(size, "medium")) |
263 | m_size = BaseImmovable::MEDIUM; |
264 | - else if (!strcasecmp(string, "big")) |
265 | + else if (iequals(size, "big")) |
266 | m_size = BaseImmovable::BIG; |
267 | else |
268 | throw GameDataError |
269 | ("expected %s but found \"%s\"", |
270 | - "{\"small\"|\"medium\"|\"big\"}", string); |
271 | + "{\"small\"|\"medium\"|\"big\"}", size); |
272 | } catch (const WException & e) { |
273 | throw GameDataError("size: %s", e.what()); |
274 | } |
275 | + } |
276 | |
277 | // parse attributes |
278 | { |
279 | |
280 | === modified file 'src/logic/save_handler.cc' |
281 | --- src/logic/save_handler.cc 2015-01-12 15:44:52 +0000 |
282 | +++ src/logic/save_handler.cc 2015-01-29 12:29:13 +0000 |
283 | @@ -21,6 +21,7 @@ |
284 | |
285 | #include <memory> |
286 | |
287 | +#include <boost/algorithm/string.hpp> |
288 | #include <boost/format.hpp> |
289 | |
290 | #include "base/log.h" |
291 | @@ -156,15 +157,7 @@ |
292 | (std::string dir, std::string filename) |
293 | { |
294 | // ok, first check if the extension matches (ignoring case) |
295 | - bool assign_extension = true; |
296 | - if (filename.size() >= strlen(WLGF_SUFFIX)) { |
297 | - char buffer[10]; // enough for the extension |
298 | - filename.copy |
299 | - (buffer, sizeof(WLGF_SUFFIX), filename.size() - strlen(WLGF_SUFFIX)); |
300 | - if (!strncasecmp(buffer, WLGF_SUFFIX, strlen(WLGF_SUFFIX))) |
301 | - assign_extension = false; |
302 | - } |
303 | - if (assign_extension) |
304 | + if (!boost::iends_with(filename, WLGF_SUFFIX)) |
305 | filename += WLGF_SUFFIX; |
306 | |
307 | // Now append directory name |
308 | |
309 | === modified file 'src/map_io/widelands_map_loader.h' |
310 | --- src/map_io/widelands_map_loader.h 2014-09-10 08:55:04 +0000 |
311 | +++ src/map_io/widelands_map_loader.h 2015-01-29 12:29:13 +0000 |
312 | @@ -20,7 +20,7 @@ |
313 | #ifndef WL_MAP_IO_WIDELANDS_MAP_LOADER_H |
314 | #define WL_MAP_IO_WIDELANDS_MAP_LOADER_H |
315 | |
316 | -#include <cstring> |
317 | +#include <boost/algorithm/string.hpp> |
318 | #include <memory> |
319 | #include <string> |
320 | |
321 | @@ -46,7 +46,7 @@ |
322 | MapObjectLoader * get_map_object_loader() {return m_mol.get();} |
323 | |
324 | static bool is_widelands_map(const std::string & filename) { |
325 | - return !strcasecmp(&filename.c_str()[filename.size() - 4], WLMF_SUFFIX); |
326 | + return boost::iends_with(filename, WLMF_SUFFIX); |
327 | } |
328 | |
329 | // If this was made pre one-world, the name of the world. |
330 | |
331 | === modified file 'src/profile/profile.cc' |
332 | --- src/profile/profile.cc 2014-10-15 08:21:03 +0000 |
333 | +++ src/profile/profile.cc 2015-01-29 12:29:13 +0000 |
334 | @@ -19,13 +19,17 @@ |
335 | |
336 | #include "profile/profile.h" |
337 | |
338 | +#include <algorithm> |
339 | #include <cctype> |
340 | #include <cstdarg> |
341 | #include <cstdio> |
342 | +#include <cstdlib> |
343 | #include <cstring> |
344 | #include <limits> |
345 | #include <string> |
346 | |
347 | +#include <boost/algorithm/string.hpp> |
348 | + |
349 | #include "base/i18n.h" |
350 | #include "base/log.h" |
351 | #include "base/wexception.h" |
352 | @@ -74,28 +78,38 @@ |
353 | |
354 | Profile g_options(Profile::err_log); |
355 | |
356 | -Section::Value::Value(const char * const nname, const char * const nval) : |
357 | - m_used(false), m_name(strdup(nname)), m_value(strdup(nval)) |
358 | -{} |
359 | +Section::Value::Value(const string & nname, const char * const nval) : |
360 | + m_used(false), |
361 | + m_name(nname) |
362 | +{ |
363 | + set_string(nval); |
364 | +} |
365 | |
366 | Section::Value::Value(const Section::Value & o) : |
367 | -m_used(o.m_used), m_name(strdup(o.m_name)), m_value(strdup(o.m_value)) {} |
368 | - |
369 | -Section::Value::~Value() |
370 | -{ |
371 | - free(m_name); |
372 | - free(m_value); |
373 | -} |
374 | - |
375 | -Section::Value & Section::Value::operator= (const Section::Value & o) |
376 | -{ |
377 | - if (this != &o) { |
378 | - free(m_name); |
379 | - free(m_value); |
380 | - m_used = o.m_used; |
381 | - m_name = strdup(o.m_name); |
382 | - m_value = strdup(o.m_value); |
383 | - } |
384 | + m_used(o.m_used), |
385 | + m_name(o.m_name) |
386 | +{ |
387 | + set_string(o.m_value.get()); |
388 | +} |
389 | + |
390 | +Section::Value::Value(Section::Value && o) |
391 | + : Value() |
392 | +{ |
393 | + using std::swap; |
394 | + swap(*this, o); |
395 | +} |
396 | + |
397 | +Section::Value & Section::Value::operator= (Section::Value other) |
398 | +{ |
399 | + using std::swap; |
400 | + swap(*this, other); |
401 | + return *this; |
402 | +} |
403 | + |
404 | +Section::Value & Section::Value::operator= (Section::Value && other) |
405 | +{ |
406 | + using std::swap; |
407 | + swap(*this, other); |
408 | return *this; |
409 | } |
410 | |
411 | @@ -112,12 +126,12 @@ |
412 | int32_t Section::Value::get_int() const |
413 | { |
414 | char * endp; |
415 | - long int const i = strtol(m_value, &endp, 0); |
416 | + long int const i = strtol(m_value.get(), &endp, 0); |
417 | if (*endp) |
418 | - throw wexception("%s: '%s' is not an integer", get_name(), m_value); |
419 | + throw wexception("%s: '%s' is not an integer", get_name(), get_string()); |
420 | int32_t const result = i; |
421 | if (i != result) |
422 | - throw wexception("%s: '%s' is out of range", get_name(), m_value); |
423 | + throw wexception("%s: '%s' is out of range", get_name(), get_string()); |
424 | |
425 | return result; |
426 | } |
427 | @@ -126,9 +140,9 @@ |
428 | uint32_t Section::Value::get_natural() const |
429 | { |
430 | char * endp; |
431 | - long long int i = strtoll(m_value, &endp, 0); |
432 | + long long int i = strtoll(m_value.get(), &endp, 0); |
433 | if (*endp || i < 0) |
434 | - throw wexception("%s: '%s' is not natural", get_name(), m_value); |
435 | + throw wexception("%s: '%s' is not natural", get_name(), get_string()); |
436 | return i; |
437 | } |
438 | |
439 | @@ -136,9 +150,9 @@ |
440 | uint32_t Section::Value::get_positive() const |
441 | { |
442 | char * endp; |
443 | - long long int i = strtoll(m_value, &endp, 0); |
444 | + long long int i = strtoll(m_value.get(), &endp, 0); |
445 | if (*endp || i < 1) |
446 | - throw wexception("%s: '%s' is not positive", get_name(), m_value); |
447 | + throw wexception("%s: '%s' is not positive", get_name(), get_string()); |
448 | return i; |
449 | } |
450 | |
451 | @@ -146,31 +160,43 @@ |
452 | bool Section::Value::get_bool() const |
453 | { |
454 | for (int32_t i = 0; i < TRUE_WORDS; ++i) |
455 | - if (!strcasecmp(m_value, trueWords[i])) |
456 | + if (boost::iequals(m_value.get(), trueWords[i])) |
457 | return true; |
458 | for (int32_t i = 0; i < FALSE_WORDS; ++i) |
459 | - if (!strcasecmp(m_value, falseWords[i])) |
460 | + if (boost::iequals(m_value.get(), falseWords[i])) |
461 | return false; |
462 | |
463 | - throw wexception("%s: '%s' is not a boolean value", get_name(), m_value); |
464 | + throw wexception("%s: '%s' is not a boolean value", get_name(), get_string()); |
465 | } |
466 | |
467 | |
468 | Point Section::Value::get_point() const |
469 | { |
470 | - char * endp = m_value; |
471 | + char * endp = m_value.get(); |
472 | long int const x = strtol(endp, &endp, 0); |
473 | long int const y = strtol(endp, &endp, 0); |
474 | if (*endp) |
475 | - throw wexception("%s: '%s' is not a Point", get_name(), m_value); |
476 | + throw wexception("%s: '%s' is not a Point", get_name(), get_string()); |
477 | |
478 | return Point(x, y); |
479 | } |
480 | |
481 | void Section::Value::set_string(char const * const value) |
482 | { |
483 | - free(m_value); |
484 | - m_value = strdup(value); |
485 | + using std::copy; |
486 | + |
487 | + const auto len = strlen(value) + 1; |
488 | + m_value.reset(new char[len]); |
489 | + copy(value, value + len, m_value.get()); |
490 | +} |
491 | + |
492 | +void swap(Section::Value & first, Section::Value & second) |
493 | +{ |
494 | + using std::swap; |
495 | + |
496 | + swap(first.m_name, second.m_name); |
497 | + swap(first.m_value, second.m_value); |
498 | + swap(first.m_used, second.m_used); |
499 | } |
500 | |
501 | |
502 | @@ -189,29 +215,9 @@ |
503 | m_section_name = name; |
504 | } |
505 | |
506 | -Section::Section(Profile * const prof, const char * const name) : |
507 | +Section::Section(Profile * const prof, const std::string & name) : |
508 | m_profile(prof), m_used(false), m_section_name(name) {} |
509 | |
510 | -Section::Section(const Section & o) : |
511 | - m_profile (o.m_profile), |
512 | - m_used (o.m_used), |
513 | - m_section_name(o.m_section_name), |
514 | - m_values (o.m_values) |
515 | -{ |
516 | - assert(this != &o); |
517 | -} |
518 | - |
519 | -Section & Section::operator= (const Section & o) { |
520 | - if (this != &o) { |
521 | - m_profile = o.m_profile; |
522 | - m_used = o.m_used; |
523 | - m_section_name = o.m_section_name; |
524 | - m_values = o.m_values; |
525 | - } |
526 | - |
527 | - return *this; |
528 | -} |
529 | - |
530 | /** Section::is_used() |
531 | * |
532 | */ |
533 | @@ -248,7 +254,7 @@ |
534 | bool Section::has_val(char const * const name) const |
535 | { |
536 | for (const Value& temp_value : m_values) { |
537 | - if (!strcasecmp(temp_value.get_name(), name)) { |
538 | + if (boost::iequals(temp_value.get_name(), name)) { |
539 | return true; |
540 | } |
541 | } |
542 | @@ -265,7 +271,7 @@ |
543 | Section::Value * Section::get_val(char const * const name) |
544 | { |
545 | for (Value& value : m_values) { |
546 | - if (!strcasecmp(value.get_name(), name)) { |
547 | + if (boost::iequals(value.get_name(), name)) { |
548 | value.mark_used(); |
549 | return &value; |
550 | } |
551 | @@ -284,7 +290,7 @@ |
552 | { |
553 | for (Value& value : m_values) { |
554 | if (!value.is_used()) { |
555 | - if (!name || !strcasecmp(value.get_name(), name)) { |
556 | + if (!name || boost::iequals(value.get_name(), name)) { |
557 | value.mark_used(); |
558 | return &value; |
559 | } |
560 | @@ -297,7 +303,7 @@ |
561 | (char const * const name, char const * const value) |
562 | { |
563 | for (Value& temp_value : m_values) { |
564 | - if (!strcasecmp(temp_value.get_name(), name)) { |
565 | + if (boost::iequals(temp_value.get_name(), name)) { |
566 | temp_value.set_string(value); |
567 | return temp_value; |
568 | } |
569 | @@ -308,8 +314,7 @@ |
570 | Section::Value & Section::create_val_duplicate |
571 | (char const * const name, char const * const value) |
572 | { |
573 | - Value v(name, value); |
574 | - m_values.push_back(v); |
575 | + m_values.emplace_back(name, value); |
576 | return m_values.back(); |
577 | } |
578 | |
579 | @@ -617,7 +622,7 @@ |
580 | Section * Profile::get_section(const std::string & name) |
581 | { |
582 | for (Section& temp_section : m_sections) { |
583 | - if (!strcasecmp(temp_section.get_name(), name.c_str())) { |
584 | + if (boost::iequals(temp_section.get_name(), name.c_str())) { |
585 | temp_section.mark_used(); |
586 | return &temp_section; |
587 | } |
588 | @@ -661,7 +666,7 @@ |
589 | { |
590 | for (Section& section : m_sections) { |
591 | if (!section.is_used()) { |
592 | - if (!name || !strcasecmp(section.get_name(), name)) { |
593 | + if (!name || boost::iequals(section.get_name(), name)) { |
594 | section.mark_used(); |
595 | return §ion; |
596 | } |
597 | @@ -674,7 +679,7 @@ |
598 | Section & Profile::create_section (char const * const name) |
599 | { |
600 | for (Section& section : m_sections) { |
601 | - if (!strcasecmp(section.get_name(), name)) { |
602 | + if (boost::iequals(section.get_name(), name)) { |
603 | return section; |
604 | } |
605 | } |
606 | |
607 | === modified file 'src/profile/profile.h' |
608 | --- src/profile/profile.h 2014-09-20 09:37:47 +0000 |
609 | +++ src/profile/profile.h 2015-01-29 12:29:13 +0000 |
610 | @@ -21,6 +21,8 @@ |
611 | #define WL_PROFILE_PROFILE_H |
612 | |
613 | #include <cstring> |
614 | +#include <memory> |
615 | +#include <string> |
616 | #include <vector> |
617 | |
618 | #include "base/macros.h" |
619 | @@ -55,17 +57,18 @@ |
620 | friend class Profile; |
621 | |
622 | struct Value { |
623 | - bool m_used; |
624 | - char * m_name; |
625 | - char * m_value; |
626 | + using string = std::string; |
627 | |
628 | - Value(char const * nname, char const * nval); |
629 | + Value(const string & name, const char * const value); |
630 | Value(const Value &); |
631 | - ~Value(); |
632 | - |
633 | - Value & operator= (const Value &); |
634 | - |
635 | - char const * get_name() const {return m_name;} |
636 | + Value(Value && other); |
637 | + |
638 | + // destructor would be empty |
639 | + |
640 | + Value & operator= (Value); |
641 | + Value & operator= (Value && other); |
642 | + |
643 | + char const * get_name() const {return m_name.c_str();} |
644 | |
645 | bool is_used() const; |
646 | void mark_used(); |
647 | @@ -74,19 +77,25 @@ |
648 | uint32_t get_natural () const; |
649 | uint32_t get_positive() const; |
650 | bool get_bool() const; |
651 | - char const * get_string() const {return m_value;} |
652 | - char * get_string() {return m_value;} |
653 | + char const * get_string() const {return m_value.get();} |
654 | + char * get_string() {return m_value.get();} |
655 | Point get_point () const; |
656 | |
657 | void set_string(char const *); |
658 | + |
659 | + friend void swap(Value& first, Value& second); |
660 | + |
661 | + private: |
662 | + bool m_used; |
663 | + string m_name; |
664 | + std::unique_ptr<char []> m_value; |
665 | + |
666 | + Value() = default; |
667 | }; |
668 | |
669 | using ValueList = std::vector<Value>; |
670 | |
671 | - Section(Profile *, char const * name); |
672 | - Section(const Section &); |
673 | - |
674 | - Section & operator= (const Section &); |
675 | + Section(Profile *, const std::string & name); |
676 | |
677 | /// \returns whether a value with the given name exists. |
678 | /// Does not mark the value as used. |
Holy cow, that is a solid first merge request. Even with an updated style checker rule. A W E S O M E!!
Very solid throughout. Just a bunch of minor nits.