Skip to content

Commit

Permalink
Fix ICUSYMNOTFOUND error message if ydb_icu_version env var is set to…
Browse files Browse the repository at this point in the history
… a long string (SS_LOG2LONG)

* The `v53003/D9I10002703` subtest in the YDBTest repo failed in in-house testing with the following
  diff when it was testing the `ydb_icu_version` env var set to a huge 32Kb sized string.

  ```diff
  30c30
  < %YDB-E-ICUSYMNOTFOUND, ... ICU needs to be built with symbol-renaming disabled or ydb_icu_version environment variable needs to be properly specified
  ---
  > %YDB-E-ICUSYMNOTFOUND, ... ICU needs to be built with symbol-renaming disabled or ydb_icu_version/gtm_icu_version environment variable needs to be properly specified
  ```

  The test was expecting the ICUSYMNOTFOUND error to indicate just the `ydb_icu_version` env var name
  as that was the env var which had been set to an incorrect value.

  But instead the actual output was `ydb_icu_verison/gtm_icu_version` which was incorrect.

* A previous commit (df110a8) fixed the ICUSYMNOTFOUND error message
  to be deterministic in case the `ydb_icu_version` and `gtm_icu_version` env vars were both not
  defined (i.e. SS_NOLOGNAM return from trans_log_name()).

  But if one of more of these env vars were set to a very huge value (i.e. SS_LOG2LONG return from
  trans_log_name()), the prior commit did not handle that case correctly. It still considered neither
  of these env vars as defined and gave an ICUSYMNOTFOUND message listing both the env var names
  whereas only one of the env var names should have been printed in that case.

  For example, if `ydb_icu_version` env var was set to a huge string (32Kb in length), then the
  ICUSYMNOTFOUND error message would print `ydb_icu_version/gtm_icu_version` instead of just
  `ydb_icu_version`. This is incorrect.

  This is now fixed by setting the `is_ydb_env_match_usable` env var to TRUE as long as the return
  from trans_log_name() is not SS_NOLOGNAM (previously it was incorrectly set to TRUE only for the
  SS_NORMAL return case).

  With these fixes, the `v53003/D9I10002703` subtest now passes.
  • Loading branch information
nars1 committed May 2, 2020
1 parent df110a8 commit 7aef327
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 10 deletions.
5 changes: 3 additions & 2 deletions sr_port/ydb_trans_log_name.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* *
* Copyright 2001, 2010 Fidelity Information Services, Inc *
* *
* Copyright (c) 2018 YottaDB LLC and/or its subsidiaries. *
* Copyright (c) 2018-2020 YottaDB LLC and/or its subsidiaries. *
* All rights reserved. *
* *
* This source code contains the intellectual property *
Expand Down Expand Up @@ -32,7 +32,8 @@ LITREF char *gtmenvname[YDBENVINDX_MAX_INDEX];
* gtmenvname[] table if the same index has a non-zero string literal and if so uses that as the environment variable
* to check if it is defined. This way, ydb* takes precedence over gtm* if both exist.
* If return is not SS_NOLOGNAM and is_ydb_env_match is non-NULL, *is_ydb_env_match is TRUE if the ydb* env var matched
* and FALSE if the gtm* env var matched.
* and FALSE if the gtm* env var matched. If return is SS_NOLOGNAM and is_ydb_env_match is non-NULL, *is_ydb_env_match
* is uninitialized and caller should not rely on this value.
* In addition, if the gtm* env var exists but the ydb* env var does not, this routine does a "setenv" of the ydb* env var
* to have the exact same value as the gtm* env var.
*/
Expand Down
14 changes: 6 additions & 8 deletions sr_unix/gtm_icu.c
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ void gtm_icu_init(void)
boolean_t is_ydb_env_match, is_ydb_env_match_usable;
mstr trans;
char *envname;
int4 trans_log_name_status;

assert(!gtm_utf8_mode);
# ifdef __MVS__
Expand Down Expand Up @@ -268,16 +269,13 @@ void gtm_icu_init(void)
* symbols without appended version numbers) will be used.
*/
ydb_icu_ver_defined = FALSE;
if (SS_NORMAL == ydb_trans_log_name(YDBENVINDX_ICU_VERSION, &trans, icu_ver_buf, SIZEOF(icu_ver_buf),
IGNORE_ERRORS_TRUE, &is_ydb_env_match))
trans_log_name_status = ydb_trans_log_name(YDBENVINDX_ICU_VERSION, &trans, icu_ver_buf, SIZEOF(icu_ver_buf),
IGNORE_ERRORS_TRUE, &is_ydb_env_match);
/* "is_ydb_env_match" is not initialized in the SS_NOLOGNAM status */
is_ydb_env_match_usable = (SS_NOLOGNAM != trans_log_name_status);
if (SS_NORMAL == trans_log_name_status)
{ /* $ydb_icu_version is defined. Do edit check on the value before considering it really defined */
ydb_icu_ver_defined = parse_ydb_icu_version(trans.addr, trans.len, icusymver, iculibver, is_ydb_env_match);
is_ydb_env_match_usable = TRUE;
} else {
/* Neither "ydb_icu_version" nor "gtm_icu_version" env vars have been specified. "is_ydb_env_match" is
* uninitialized at this point.
*/
is_ydb_env_match_usable = FALSE;
}
if (ydb_icu_ver_defined)
{ /* User explicitly specified an ICU version. So load version specific icu file (e.g. libicuio.so.36) */
Expand Down

0 comments on commit 7aef327

Please sign in to comment.