Drizzle

Merge lp://qastaging/~vjsamuel/drizzle/fix-casts-drizzle-client into lp://qastaging/drizzle/7.0

fix-casts-drizzle-client
Merge into 7.0

Proposed by Vijay Samuel on 2011-02-23

Status:

Work in progress

Proposed branch:

lp://qastaging/~vjsamuel/drizzle/fix-casts-drizzle-client

Merge into:

lp://qastaging/drizzle/7.0

Diff against target:

247 lines (+33/-33)

1 file modified

client/drizzle.cc (+33/-33)

To merge this branch:

bzr merge lp://qastaging/~vjsamuel/drizzle/fix-casts-drizzle-client

Related bugs:

Bug #717279: C style casts

Wishlist

Confirmed

Link a bug report

Reviewer	Date Requested	Status
Olaf van der Spek	2011-02-23	Pending
Drizzle Developers	2011-02-23	Pending
Review via email: mp+50969@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2011-02-23.

Revision history for this message

Olaf van der Spek (olafvdspek) wrote on 2011-02-23: Posted in a previous version of this proposal

> if (!(histfile_tmp= (char*) malloc(static_cast<uint32_t>(strlen(histfile) + 5))))
> const char * start = const_cast<const char *>(set);
> return (const_cast<const char *>(str));

Are these casts necessary at all?

review: Needs Fixing

Revision history for this message

Monty Taylor (mordred) wrote on 2011-02-23: Posted in a previous version of this proposal

- if (!(histfile_tmp= (char*) malloc(static_cast<uint32_t>(strlen(histfile) + 5))))

The internal one (static_cast<uint32_t> is not needed. strlen returns size_t which is the type malloc takes. However, (char *) can certainly be changed to use a c++ cast. (and don't even get me started on this needing to be a std::string instead of a malloc'd char *...

The other two are not - you do not need to const cast to _add_ const qualifiers, only to take them away.

Revision history for this message

Olaf van der Spek (olafvdspek) wrote on 2011-02-23:

Nice work, but some comments:

On Wed, Feb 23, 2011 at 7:17 PM, Vijay Samuel <email address hidden> wrote:
> - tmp= (char *) getenv("DRIZZLE_HOST");
> + tmp= const_cast<char *>(getenv("DRIZZLE_HOST"));

Why? Can't tmp be made const char*?

> - if (!((char*) (pagpoint)))
> + if (!(const_cast<char *>((pagpoint))))

Why? pagpoint is already non-const.

> - len=(uint32_t) (end - name);
> + len=static_cast<uint32_t>((end - name));

len should probably be size_t so these casts aren't necessary.

> - char *end_of_line=line+(uint32_t) strlen(line);
> + char *end_of_line= line + static_cast<uint32_t>(strlen(line));

Why? It might be useful to check whether a cast is necessary in the first place.

Greetings,

Olaf

Unmerged revisions

2196. By Vijay Samuel on 2011-02-23: Merge removed unnecessary const casts
2195. By Vijay Samuel on 2011-02-23: Merge C++ style casts for drizzle client