[Tarantool-patches] [PATCH v2 1/2] Refactor decNumberFromString
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Thu Jun 25 01:22:41 MSK 2020
Hi! Thanks for the patch!
Technically looks fine, although I didn't test it except for
the tests we already have. You probably should create a patchset
for the main repository with decnumber version bump + decimal.h
API update + unit tests in unit/decimal.c. Before this commit
is merged, so as you could fix any potential errors.
> diff --git a/decNumber.c b/decNumber.c
> index 85deb13..e248656 100644
> --- a/decNumber.c
> +++ b/decNumber.c
> @@ -545,7 +545,7 @@ char * decNumberToEngString(const decNumber *dn, char *string){
> /* */
> /* If bad syntax is detected, the result will be a quiet NaN. */
> /* ------------------------------------------------------------------ */
> -decNumber * decNumberFromString(decNumber *dn, const char chars[],
> +const char * decNumberFromString(decNumber *dn, const char chars[],
> decContext *set) {
1. The code style is absolutely dead, but it is consistently dead.
Part of its consistency was the multi-line argument alignment, the
same as in Tarantool. Here the second line should be aligned under
the first argument of the first line:
const char * decNumberFromString(decNumber *dn, const char chars[],
decContext *set) {
Not like this:
const char * decNumberFromString(decNumber *dn, const char chars[],
decContext *set) {
> diff --git a/decNumber.h b/decNumber.h
> index 85a3f3f..ffaa3d8 100644
> --- a/decNumber.h
> +++ b/decNumber.h
> @@ -101,19 +101,19 @@
> /* decNumber public functions and macros */
> /* ---------------------------------------------------------------- */
> /* Conversions */
> - decNumber * decNumberFromInt32(decNumber *, int32_t);
> - decNumber * decNumberFromUInt32(decNumber *, uint32_t);
> - decNumber * decNumberFromInt64(decNumber *, int64_t);
> - decNumber * decNumberFromUInt64(decNumber *, uint64_t);
> - decNumber * decNumberFromString(decNumber *, const char *, decContext *);
> - char * decNumberToString(const decNumber *, char *);
> - char * decNumberToEngString(const decNumber *, char *);
> - uint32_t decNumberToUInt32(const decNumber *, decContext *);
> - uint64_t decNumberToUInt64(const decNumber *, decContext *);
> - int32_t decNumberToInt32(const decNumber *, decContext *);
> - int64_t decNumberToInt64(const decNumber *, decContext *);
> - uint8_t * decNumberGetBCD(const decNumber *, uint8_t *);
> - decNumber * decNumberSetBCD(decNumber *, const uint8_t *, uint32_t);
> + decNumber * decNumberFromInt32(decNumber *, int32_t);
> + decNumber * decNumberFromUInt32(decNumber *, uint32_t);
> + decNumber * decNumberFromInt64(decNumber *, int64_t);
> + decNumber * decNumberFromUInt64(decNumber *, uint64_t);
> + const char * decNumberFromString(decNumber *, const char *, decContext *);
> + char * decNumberToString(const decNumber *, char *);
> + char * decNumberToEngString(const decNumber *, char *);
> + uint32_t decNumberToUInt32(const decNumber *, decContext *);
> + uint64_t decNumberToUInt64(const decNumber *, decContext *);
> + int32_t decNumberToInt32(const decNumber *, decContext *);
> + int64_t decNumberToInt64(const decNumber *, decContext *);
> + uint8_t * decNumberGetBCD(const decNumber *, uint8_t *);
> + decNumber * decNumberSetBCD(decNumber *, const uint8_t *, uint32_t);
2. Lets not do so many changes. I think it is ok to break
the alignment in one line. Or you can wrap function name on
a next line, if you want.
So as it would be
decNumber * decNumberFromUInt64(decNumber *, uint64_t);
const char *
decNumberFromString(decNumber *, const char *, decContext *);
char * decNumberToString(const decNumber *, char *);
Or break the alignment:
decNumber * decNumberFromUInt64(decNumber *, uint64_t);
const char * decNumberFromString(decNumber *, const char *, decContext *);
char * decNumberToString(const decNumber *, char *);
But I really don't like when alignment causes so many changes.
More information about the Tarantool-patches
mailing list