gh-libtom/libtomcrypt

Watch

Fork 0

Add -Wwrite-strings (char* vs. const char*) #290

Merged

karel-m merged 2 commits from pr/write-strings into release/1.18.0

2017-09-19 06:24:14 -05:00

karel-m commented

2017-09-15 03:17:28 -05:00

(Migrated from github.com)

This PR fixes warnings which appeared after turning on -Wwrite-strings.

But frankly, I am not sure whether we want or need to apply this.

This PR fixes warnings which appeared after turning on `-Wwrite-strings`. But frankly, I am not sure whether we want or need to apply this.

sjaeckel (Migrated from github.com) approved these changes

2017-09-16 17:35:24 -05:00

sjaeckel (Migrated from github.com) left a comment

Cool PR 👍

Cool PR :+1:

sjaeckel commented

2017-09-16 17:39:09 -05:00

(Migrated from github.com)

But frankly, I am not sure whether we want or need to apply this.

somehow you're right, we should probably wait until the release is done to apply this... not sure though... also in regards to ABI breakage...

> But frankly, I am not sure whether we want or need to apply this. somehow you're right, we should probably wait until the release is done to apply this... not sure though... also in regards to ABI breakage...

sjaeckel commented

2017-09-16 17:42:57 -05:00

(Migrated from github.com)

@lvc could you please already tell us how this would influence our score? :-)

karel-m commented

2017-09-17 08:29:55 -05:00

(Migrated from github.com)

As for the API breakage I have doubts about changes in ltc_ecc_set_type the rest IMO should not cause big troubles.

As for the API breakage I have doubts about changes in `ltc_ecc_set_type` the rest IMO should not cause big troubles.

karel-m commented

2017-09-18 02:20:56 -05:00

(Migrated from github.com)

Suggestion: let's merge to release/1.18.0 the changes in tests/* + changes in <name>_test() functions. These are not going to break anything. After that we can discuss what will be left.

@sjaeckel what do you think?

Suggestion: let's merge to `release/1.18.0` the changes in `tests/*` + changes in `<name>_test()` functions. These are not going to break anything. After that we can discuss what will be left. @sjaeckel what do you think?

sjaeckel commented

2017-09-18 02:28:00 -05:00

(Migrated from github.com)

K, then split the commits up please.

karel-m commented

2017-09-18 02:32:31 -05:00

(Migrated from github.com)

Done #292

Once we have OK from Travis-CI I'll merge #292 to release/1.18.0.

Done #292 Once we have OK from Travis-CI I'll merge #292 to `release/1.18.0`.

karel-m commented

2017-09-18 05:39:10 -05:00

(Migrated from github.com)

Merged & rebased.

Now looking at the rest I think it is pretty safe to also merge:

changes to ltc_dh_set_type (DH is completely new in 1.18)
changes to crypt_argchk

Merged & rebased. Now looking at the rest I think it is pretty safe to also merge: * changes to `ltc_dh_set_type` (DH is completely new in 1.18) * changes to `crypt_argchk`

sjaeckel (Migrated from github.com) approved these changes

2017-09-19 06:23:40 -05:00

sjaeckel (Migrated from github.com) left a comment