search

YottaDB / YDB

Mirrored from https://gitlab.com/YottaDB/DB/YDB
Other
76 stars 37 forks source link

Simpleapi #190

Closed nars1 closed 6 years ago

ksbhaskar commented 6 years ago

On 03/24/2018 11:26 PM, Steven E. Estes wrote:

@estess requested changes on this pull request.

Took quite a chunk of time but got through it.

In sr_port/mdef.h https://github.com/YottaDB/YottaDB/pull/190#discussion_r176920861:

define MICROSECS_IN_MSEC 1000 / microseconds in a millisecond /

+#define NANOSEC_IN_SEC 1000000000 / nanoseconds in a second /

Seems like a duplicate of the next item down. Both are "nanoseconds in a second".

[KSB] It seems to me that defining the constant as 1E9 is less prone than 1000000000 to the possibility of an inadvertent extra or missing zero.

– Bhaskar

In sr_port/merrors.msg https://github.com/YottaDB/YottaDB/pull/190#discussion_r176920893:

@@ -649,7 +649,7 @@ TPLOCK <Cannot release LOCK(s) held prior to current TSTART>/error/fao=0!/ansi= TPQUIT /error/fao=0!/ansi=42 TPFAIL <Transaction COMMIT failed. Failure code: !AD.>/error/fao=2!/ansi=0 TPRETRY /error/fao=0!/ansi=0 -TPTOODEEP <$TLEVEL cannot exceed 127>/error/fao=0!/ansi=0 +TPTOODEEP <$TLEVEL cannot exceed 126>/error/fao=0!/ansi=0

Should the 126 be a !UL with substitution of some #define that I assume exists somewhere? That way if it changes, we only need worry about changing the #define.

In sr_port/merrors.msg https://github.com/YottaDB/YottaDB/pull/190#discussion_r176920926:

@@ -1166,7 +1166,7 @@ DLLVERSION <Routine !AD in library !AD was compiled with an incompatible version INVZROENT <!AD is neither a directory nor an object library(DLL)>/error/fao=2!/ansi=0 DDPLOGERR <!AD: !AD>/error/fao=4!/ansi=0 GETSOCKNAMERR <Getting the socket name failed from getsockname(): (errno==!UL) !AD>/error/fao=3!/ansi=0 -INVGTMEXIT /error/fao=0!/ansi=0 +INVYDBEXIT /error/fao=0!/ansi=0

ydb_exit -> ydb_exit()? Makes it clear we are talking about a call. Also perhaps start the 2nd sentence with "Calls to ydb_exit() cannot be made..." so we can capitalize the first word.

In sr_port/merrors.msg https://github.com/YottaDB/YottaDB/pull/190#discussion_r176920977:

@@ -1327,7 +1327,7 @@ BADTAG <Unable to use file !AD (CCSID !UL) with CCSID !UL>/error/fao=4!/ansi=0 ICUVERLT36 <!AD !UL.!UL. ICU version greater than or equal to 3.6 should be used>/error/fao=4!/ansi=0 ICUSYMNOTFOUND <Symbol !AD not found in the ICU libraries. ICU needs to be built with symbol-renaming disabled or gtm_icu_version environment variable needs to be properly specified>/error/fao=2!/ansi=0 STUCKACT <Process stuck script invoked: !AD : !AD>/info/fao=4!/ansi=0 -CALLINAFTERXIT <After a gtm_exit, a process can never create a valid YottaDB context>/error/fao=0!/ansi=0 +CALLINAFTERXIT <After a ydb_exit, a process can never create a valid YottaDB context>/error/fao=0!/ansi=0

Same ydb_exit -> ydb_exit(). Also "can never" -> "cannot". Never is a long time.. :-). I know these are the original texts (this and the above comment) but that doesn't mean they can't be improved. Might as well take advantage of the change we are already making.

In sr_port/merrors_ctl.c https://github.com/YottaDB/YottaDB/pull/190#discussion_r176921000:

@@ -1513,6 +1513,6 @@ LITDEF err_msg merrors[] = {

GBLDEF err_ctl merrors_ctl = { 246,

  • "YDB",
  • "GTM",

Any idea why this was changed back so as to not default to YDB? I don't see a change in the repository at the top of merrors.msg.

In sr_port/op_fnquery.c https://github.com/YottaDB/YottaDB/pull/190#discussion_r176921088:

if (NULL == lvt)

{

  • if (is_simpleapi_mode)

Though this was probably my change, it can be simplified by using !is_simpleapi_mode to set the dst parm to null string and then common return.

In sr_port/op_fnquery.c https://github.com/YottaDB/YottaDB/pull/190#discussion_r176921122:

@@ -228,6 +257,8 @@ void op_fnquery_va(int sbscnt, mval *dst, va_list var) assert(h1 >= &history[0]); if (h1 == &history[0]) {

  • if (is_simpleapi_mode)

Same as previous comment

In sr_port/op_fnquery.c https://github.com/YottaDB/YottaDB/pull/190#discussion_r176921215:

  • v2->mvtype = 0; /* initialize it to 0 to avoid "stp_gcol" from getting confused
    • if it gets invoked before v2 has been completely setup. */
  • if (TREF(local_collseq))
  • {
  • ALLOC_XFORM_BUFF(mv->str.len);
  • assert(NULL != TREF(lcl_coll_xform_buff));
  • tmp_sbs.str.addr = TREF(lcl_coll_xform_buff);
  • tmp_sbs.str.len = TREF(max_lcl_coll_xform_bufsiz);
  • do_xform(TREF(local_collseq), XBACK, &mv->str, &tmp_sbs.str, &length);
  • tmp_sbs.str.len = length;
  • v2->str = tmp_sbs.str;
  • /* The actual return of the next node differs significantly depending on whether this is a $QUERY() call from
    • generated code or a ydb_node_next_s() call from the simpleAPI. Fork that difference here.
  • */
  • if (is_simpleapi_mode)
  • { /* SimpleAPI mode - return a list of subscripts in an array of ydb_buffer_t structures which have pre-allocated

Incorrect - this routine returns (sets into a global var) a list of mstrs that are later used to populate the caller's passed in ydb_buffer_t array.

In sr_port/op_fnreversequery.c https://github.com/YottaDB/YottaDB/pull/190#discussion_r176921296:

  • v2->mvtype = 0; /* initialize it to 0 to avoid "stp_gcol" from getting confused
    • if it gets invoked before v2 has been completely setup. */
  • if (TREF(local_collseq))
  • {
  • ALLOC_XFORM_BUFF(mv->str.len);
  • assert(NULL != TREF(lcl_coll_xform_buff));
  • tmp_sbs.str.addr = TREF(lcl_coll_xform_buff);
  • tmp_sbs.str.len = TREF(max_lcl_coll_xform_bufsiz);
  • do_xform(TREF(local_collseq), XBACK, &mv->str, &tmp_sbs.str, &length);
  • tmp_sbs.str.len = length;
  • v2->str = tmp_sbs.str;
  • /* The actual return of the next node differs significantly depending on whether this is a $QUERY() call from
    • generated code or a ydb_node_next_s() call from the simpleAPI. Fork that difference here.
  • */
  • if (is_simpleapi_mode)
  • { /* SimpleAPI mode - return a list of subscripts in an array of ydb_buffer_t structures which have pre-allocated

Same problem here as in op_fnquery.c - This routine returns (in a C global variable) a list of mstr blocks describing the subscripts to return. These are later (different routine) used to populated the caller's ydb_buffer_t array.

In sr_port/outofband_action.c https://github.com/YottaDB/YottaDB/pull/190#discussion_r176921657:

            break;

case (tptimeout):

  • /*
    • Currently following is nothing but an rts_error.
    • Function pointer is used flexibility.
  • /* Currently following is nothing but an rts_error. Function pointer is used flexibility.
  • *
    • Note this is the only outofband currently allowed for simpleAPI functions.

Last note is no longer true. Ctrl-C is also allowed - which exits in simpleAPI mode.

In sr_port/push_lvval.c https://github.com/YottaDB/YottaDB/pull/190#discussion_r176921727:

@@ -34,12 +37,6 @@ lv_val push_lvval(mval arg1) error_def(ERR_STACKOFLOW); error_def(ERR_STACKCRIT);

  • /* Note that since this is only (currently) used by call-ins and no TP
  • transaction can be (currently) be active during a call-in, we do not
  • worry about setting up tp_var structures as is done in lv_newname().
  • The assert below will catch if this condition changes.
  • */
  • assert(!dollar_tlevel);

Both the warning and the assert verification were removed but I don't see anything mitigating what the comment warned about. Was the comment wrong?

In sr_port/stp_gcol_src.h https://github.com/YottaDB/YottaDB/pull/190#discussion_r176921821:

@@ -838,21 +838,30 @@ void stp_gcol(size_t space_asked) / BYPASSOK / */ if (NULL != frame_pointer) {

  • for (sf = frame_pointer; sf < (stack_frame *)stackbase; sf = sf->old_frame_pointer)
  • { / Cover temp mvals in use /
  • if (NULL == sf->old_frame_pointer)
  • { / If trigger enabled, may need to jump over a base frame /
  • / TODO - fix this to jump over call-ins base frames as well / -# ifdef GTM_TRIGGER
  • if (SFT_TRIGR & sf->type)
  • { / We have a trigger base frame, back up over it /
  • for (sf = frame_pointer; sf && (sf < (stack_frame *)stackbase); sf = sf->old_frame_pointer)
  • { /* Move temp mvals in use to stringpool
  • *
    • While running through the stack, we need to skip over base frames so we process the
    • entire stack with the following caveats:
      1. The original base frame (has a type of SFT_COUNT but is otherwise unmarked) is the
    • absolute bottom of the stack so breaks the loop. This will only be seen when the

Some broken indenting in these comments.

In sr_port/tp_restart.c https://github.com/YottaDB/YottaDB/pull/190#discussion_r176921957:

@@ -193,12 +191,10 @@ int tp_restart(int newlevel, boolean_t handle_errors_internally) rts_error_csa(CSA_ARG(NULL) VARLSTCNT(1) ERR_TLVLZERO); return 0; / for the compiler only -- never executed / } -# ifdef GTM_TRIGGER

I wasn't aware we were removing TRIGGER type ifdefs. Are we assuming all UNIX platforms from here out will be able to run triggers? (not sure that's true for cygwin or not) - new architectures are also possible - especially in the IOT arena (IBM just announced one with a CPU the size of a grain of salt with the power of 1990s CPUs - and they apparently stack. There was a picture of the guy holding a square of silicon maybe a centimeter square that had 64 "motherboards" on it and 2 cpus. I guess it gets broken apart by an ultra thin laser or something. The cost of each CPU is 10 cents.

In sr_port/tpdefs.c https://github.com/YottaDB/YottaDB/pull/190#discussion_r176922005:

@@ -17,6 +20,9 @@ GBLDEF uint4 dollar_tlevel; GBLDEF uint4 bml_save_dollar_tlevel; / if non-zero holds actual dollar_tlevel / GBLDEF uint4 dollar_trestart; +GBLDEF uint4 simpleapi_dollar_trestart; /* copy of dollar_trestart taken by "ydb_tp_s",

Does this need to be a threadgbl to support nested TP levels? It certainly will need to be converted to a global var to do the multi-threaded project at some point.

In sr_port/ydberrors_ansi.h https://github.com/YottaDB/YottaDB/pull/190#discussion_r176922060:

@@ -18,4 +18,18 @@ const static readonly int error_ansi[] = { 0, / LIBYOTTAMISMTCH /

Is this module supposed to be in there? I don't think it is. We keep deleting it but it keeps popping back up because the *.msg commit code regenerates it though it does not commit it. If you aren't careful, the next commit ends up containing it.

In sr_port/zshow_stack.c https://github.com/YottaDB/YottaDB/pull/190#discussion_r176922092:

@@ -30,7 +30,7 @@

define ZINTR_FRAME " ($ZINTERRUPT) "

define DEVERR_FRAME " (Device Error)"

define DIR_MODE_MESS " (Direct mode) "

-#define CALL_IN_BASE_FRAME " (Call-In Level Entry)" +#define CALL_IN_BASE_FRAME " " CALL_IN_M_ENTRYREF

Not understanding why this was changed to something that looks a lot uglier than it was. I'd suggest changing it to "(Call-In Entry)" or just "(Call-In)" instead of some cryptic thing full of underbars.

In sr_unix/errorsp.h https://github.com/YottaDB/YottaDB/pull/190#discussion_r176922334:

@@ -451,12 +461,12 @@ MBSTART { \ CHTRACEPOINT; \ DEFER_INTERRUPTS(INTRPT_IN_CONDSTK, prev_intrpt_state); \ chnd[current_ch].ch_active = FALSE; \

  • assert((active_ch+1)->dollar_tlevel == dollar_tlevel); \

Spaces around '+' sign.

In sr_unix/ftok_sems.h https://github.com/YottaDB/YottaDB/pull/190#discussion_r176922365:

@@ -19,6 +22,9 @@

define DECR_CNT_TRUE TRUE

define DECR_CNT_SAFE 2

+#define IMMEDIATE_FALSE FALSE

Purpose (comment)?

In sr_unix/gtm_semutils.h https://github.com/YottaDB/YottaDB/pull/190#discussion_r176926497:

@@ -189,6 +192,16 @@ MBSTART { \ SOP[0].sem_flg = SOP[1].sem_flg = SOP[2].sem_flg = SEMFLG; \ }

+#define SET_SOP_ARRAY_FOR_INCR_CNT(SOP, SOPCNT, SEMFLG) \ +{ \

  • /* Typically, multiple statements are not specified in a single line. However, each of the 2 lines below represent \

Does this comment need work? It doesn't seem to make much sense. There are no multiple statements per line in this macro. Also, should it be using MBSTART/MBEND?

In sr_unix/gtmci.c https://github.com/YottaDB/YottaDB/pull/190#discussion_r176926586:

@@ -571,6 +572,17 @@ int ydb_ci_exec(const char c_rtn_name, void callin_handle, int populate_handle SETUP_THREADGBL_ACCESS; VAR_COPY(var, temp_var); added = FALSE;

  • / Do the "ydb_init" (if needed) first as it would set gtm_threadgbl etc. which is needed to use TREF later /

Not an issue for now but a thought - when we go to a threaded model, how are we going to test gtm_startup_active and frame_pointer - both of which need to be threadgbls? Perhaps the first test should be whether lcl_gtm_threadgbl is non-NULL?

In sr_unix/libyottadb.h https://github.com/YottaDB/YottaDB/pull/190#discussion_r176926667:

+#ifndef NULL +# define NULL ((void )0) +#endif + +/ Macro to create/fill-in a ydb_buffer_t structure from a literal - use - literal varnames, subscripts

    • or values.
  • */ +#define YDB_LITERAL_TO_BUFFER(LITERAL, BUFFERP) \ +{ \
  • (BUFFERP)->buf_addr = LITERAL; \
  • (BUFFERP)->len_used = (BUFFERP)->len_alloc = sizeof(LITERAL) - 1; \ +}

  • +/ Below macro returns TRUE if two input ydb_buffer_t structures pointer to the same string and FALSE otherwise. / +#define YDB_BUFFER_IS_SAME(BUFFERP1, BUFFERP2) \

  • ((BUFFERP1->len_used == BUFFERP2->len_used) && !memcmp(BUFFERP1->buf_addr, BUFFERP2->buf_addr, BUFFERP2->len_used))

BUFFERP1 and BUFFERP2 usages in this line should be surrounded by parens .. e.g. (BUFFERP1)->len_used.

In sr_unix/libyottadb.h https://github.com/YottaDB/YottaDB/pull/190#discussion_r176926696:

  • } \ +}

  • +/* Macro to determine severity of error returned from simpleapi call. Note a possible return value is an errno value. These

    • errno values do not follow the same rules as YottaDB generated errors so this macro does not work on them. YottaDB
    • generated errors numbers are all (absolute value) LARGER than 2**27 so anything under that is not supported by this macro.
  • */ +#define YDB_SEVERITY(MSGNUM, SEVERITY) \ +{ \
  • /* Minor subtrifuge because YDB_OK is 0 (per normal UNIX return code) but the rest of the codes, when the \
    • error is out of the range of errno values, have 1 as a success value. \
  • */ \
  • if (YDB_OK == (MSGNUM)) \
  • SEVERITY = YDB_SEVERITY_SUCCESS; \
  • else \
  • SEVERITY = ((int)abs(MSGNUM) & 7); / Negation turns msg code into actual code used internally / \

(My) Comment needs to change. No longer just negating value but doing abs(value) so always have positive version.

In sr_unix/libyottadb_int.h https://github.com/YottaDB/YottaDB/pull/190#discussion_r176926875:

    • there is also no executable frame (and likewise during initialization of a call-in before the executable
    • frame has been setup. As long as this macro is only used in places where we know we are dealing with a
    • runtime call (i.e. op_*), then this macro is accurate.
  • */ +#define IS_SIMPLEAPI_MODE (frame_pointer->type & SFT_CI)

  • +#define ISSUE_TIME2LONG_ERROR_IF_NEEDED(INPUT_TIME_IN_NANOSECONDS) \ +MBSTART { \

  • unsigned long long max_time_nsec; \
  • \
  • assert(SIZEOF(unsigned long long) == SIZEOF(INPUT_TIME_IN_NANOSECONDS)); \
  • assert(MAXPOSINT4 == (YDB_MAX_TIME_NSEC / NANOSECS_IN_MSEC)); \
  • if (YDB_MAX_TIME_NSEC < INPUT_TIME_IN_NANOSECONDS) \
  • { \
  • max_time_nsec = YDB_MAX_TIME_NSEC; \
  • rts_error_csa(CSA_ARG(NULL) VARLSTCNT(4) ERR_TIME2LONG, 2, \

This TIME2LONG error is not correct but the entire macro seems to be removed in the latest version I find.

In sr_unix/make_dmode.c https://github.com/YottaDB/YottaDB/pull/190#discussion_r176926893:

  • base_address->routine_name.addr = dmode->rtn_name;
  • base_address->ptext_adr = (unsigned char *)base_address + algnd_rtnhdr_size;
  • base_address->ptext_end_adr = (unsigned char *)base_address->ptext_adr + algnd_code_size;
  • base_address->lnrtab_adr = (lnr_tabent *)base_address->ptext_end_adr;
  • base_address->labtab_adr = (lab_tabent )((unsigned char )base_address + algnd_rtnhdr_size +
  • algnd_code_size + algnd_lnrtab_size);
  • base_address->lnrtab_len = CODE_LINES;
  • base_address->labtab_len = 1;
  • code = (CODEBUF_TYPE )base_address->ptext_adr; / start of executable code */
  • GEN_CALL(dmode->func_ptr1); / line 0,1 /
  • GEN_CALL(dmode->func_ptr2);
  • GEN_CALL(dmode->func_ptr3); / line 2 /
  • lnr = LNRTAB_ADR(base_address);
  • lnr++ = 0; / line 0 */
  • lnr++ = 0; / line 1 */
  • IA64_ONLY(lnr++ = 2 CALL_SIZE + EXTRA_INST_SIZE;) / line 2 /

Any purpose in keeping these here (IA64 is not even supported by pretty much anyone anymore).

In sr_unix/make_mode.c https://github.com/YottaDB/YottaDB/pull/190#discussion_r176926913:

@@ -1,101 +0,0 @@ -/****

Don't really understand why this was renamed. True, it does only support creating the YBB$DMOD module but it wouldn't be surprising to find another need for this at which point we'll just have to rename it back.

In sr_unix/gtm_unique_file_util.c https://github.com/YottaDB/YottaDB/pull/190#discussion_r176927285:

boolean_t   status;

gd_id_ptr_t tmp_fileid;

  if (!filename)
      return FALSE;
  assert(fileid && !*fileid);
  tmp_fileid = (gd_id_ptr_t)malloc(SIZEOF(gd_id));
  • status = (SS_NORMAL == filename_to_id(tmp_fileid, filename->address));
  • *fileid = (xc_fileid_ptr_t)tmp_fileid;
  • return status;
  • actstatus = filename_to_id(tmp_fileid, filename->address);
  • status = (SS_NORMAL == actstatus);
  • if (status)
  • *fileid = (ydb_fileid_ptr_t)tmp_fileid;
  • else
  • { / There was an error /
  • free(tmp_fileid);
  • *fileid = NULL;
  • }
  • return status ? YDB_OK : actstatus;

My change but realize can't change the way THIS routine operates. It still needs to return true for success or false for failure but that means there's no way to return the actual error back to ydb_file_name_toid (say caller passed garbage as a filename address or a ridiculous string length). This change works for the ydb flavor but breaks the gtm_ flavor. Quandry..

In sr_unix/ydb_file_is_identical.c https://github.com/YottaDB/YottaDB/pull/190#discussion_r176927320:

+

  • SETUP_THREADGBL_ACCESS;
  • if (process_exiting)
  • { / YDB runtime environment not setup/available, no driving of errors /
  • send_msg_csa(CSA_ARG(NULL) VARLSTCNT(1) ERR_CALLINAFTERXIT);
  • return YDB_ERR_CALLINAFTERXIT;
  • }
  • ESTABLISH_NORET(ydb_simpleapi_ch, error_encountered);
  • if (error_encountered)
  • { / Some error occurred - return the error code to the caller ($ZSTATUS is set) /
  • REVERT;
  • return -(TREF(ydb_error_code));
  • }
  • status = gtm_is_file_identical(fileid1, fileid2);
  • REVERT;
  • return status ? YDB_OK : -1;

Spec does not define what not-identical return value is. The code is currently returning -1 but something less hard-coded would be a better choice - also something defined in the spec. Same issue exists with ydb_thread_is_main().

In sr_unix/ydb_file_name_to_id.c https://github.com/YottaDB/YottaDB/pull/190#discussion_r176927343:

  • ydb_status_t status;
  • DCL_THREADGBL_ACCESS;
  • SETUP_THREADGBL_ACCESS;
  • ESTABLISH_NORET(ydb_simpleapi_ch, error_encountered);
  • if (process_exiting)
  • { / YDB runtime environment not setup/available, no driving of errors /
  • send_msg_csa(CSA_ARG(NULL) VARLSTCNT(1) ERR_CALLINAFTERXIT);
  • return YDB_ERR_CALLINAFTERXIT;
  • }
  • if (error_encountered)
  • { / Some error occurred - return the error code to the caller ($ZSTATUS is set) /
  • REVERT;
  • return -(TREF(ydb_error_code));
  • }
  • status = gtm_filename_to_id(filename, fileid);

See comments for gtm_filename_to_id() above - we can only get a true/false return value out of it without breaking it.

In sr_unix/ydb_thread_is_main.c https://github.com/YottaDB/YottaDB/pull/190#discussion_r176927454:

+

  • SETUP_THREADGBL_ACCESS;
  • if (process_exiting)
  • { / YDB runtime environment not setup/available, no driving of errors /
  • send_msg_csa(CSA_ARG(NULL) VARLSTCNT(1) ERR_CALLINAFTERXIT);
  • return YDB_ERR_CALLINAFTERXIT;
  • }
  • ESTABLISH_NORET(ydb_simpleapi_ch, error_encountered);
  • if (error_encountered)
  • { / Some error occurred - return the error code to the caller ($ZSTATUS is set) /
  • REVERT;
  • return -(TREF(ydb_error_code));
  • }
  • status = gtm_is_main_thread();
  • REVERT;
  • return status ? YDB_OK : -1;

As noted above in ydb_file_is_identical(), the -1 return code needs to be less hardcoded - probably given a #define in libyottadb.h and that name needs to be documented.

In sr_unix/ydb_tp_s.c https://github.com/YottaDB/YottaDB/pull/190#discussion_r176927502:

  • op_trestart_set_cdb_code();
  • }
  • rc = tp_restart(1, !TP_RESTART_HANDLES_ERRORS);
  • assert((0 == rc) && (TPRESTART_STATE_NORMAL == tprestart_state));
  • }
  • tpfn_status = YDB_OK; / Now that the restart processing is complete, clear status back to normal /
  • }
  • }
  • LIBYOTTADB_DONE; / Shutoff active rtn indicator while call callback routine /
  • if (!save_dollar_tlevel)
  • { /* This is the outermost TP. Note down "simpleapi_dollar_trestart" for later use
    • by "ydb_tp_s"/"ydb_simpleapi_ch" to help decide whether "tp_restart" processing is needed.
  • */
  • simpleapi_dollar_trestart = dollar_trestart;
  • }
  • if (YDB_OK == tpfn_status)

Would be nice to have a comment here that we are driving the transaction routine supplied by the user here.

In sr_unix/ydbinstall.sh https://github.com/YottaDB/YottaDB/pull/190#discussion_r176927554:

@@ -136,10 +136,10 @@ help_exit() echo "version is defaulted from mumps file if one exists in the same directory as the installer" echo "This version must run as root." echo ""

  • echo "Example usages are (assumes latest YottaDB release is r1.10 and latest GT.M version is V6.3-002)"
  • echo " ydbinstall.sh # installs latest YottaDB release (r1.10) at /usr/local/lib/yottadb/r110"
  • echo " ydbinstall.sh --utf8 default # installs YottaDB release r1.10 with added support for UTF-8"
  • echo " ydbinstall.sh --installdir /r110 r1.10 # installs YottaDB r1.10 at /r110"
  • echo "Example usages are (assumes latest YottaDB release is r1.20 and latest GT.M version is V6.3-002)"
  • echo " ydbinstall.sh # installs latest YottaDB release (r1.20) at /usr/local/lib/yottadb/r120"
  • echo " ydbinstall.sh --utf8 default # installs YottaDB release r1.20 with added support for UTF-8"
  • echo " ydbinstall.sh --installdir /r120 r1.20 # installs YottaDB r1.20 at /r120" echo " ydbinstall.sh --gtm # installs latest GT.M version (V6.3-002) at /usr/local/lib/fis-gtm/V6.3-002_x86_64"

Are the V6.3-002 comments correct? I think we are doing V6.3-003A now right? I haven't looked deeper but assume there are other V6.3-002 references in here that should probably be corrected.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/YottaDB/YottaDB/pull/190#pullrequestreview-106711290, or mute the thread https://github.com/notifications/unsubscribe-auth/AAiqep2bKTrWbvUgOAQCrWqGFLLlI94tks5thw55gaJpZM4S5F4U.

-- YottaDB - Rock solid. Lightning fast. Secure. Pick any three.

nars1 commented 6 years ago

@ksbhaskar : Yes 1E9 is less error prone. Now fixed there and other similar places in the same module.

nars1 commented 6 years ago

@estess : I am done addressing all review comments. There are 4 comments which I expect you will own since you were the author. Let me know if I missed any comments.

YottaDB commented 6 years ago

On 03/25/2018 03:36 PM, Steven E. Estes wrote:

@estess commented on this pull request.

In sr_unix/gtm_unique_file_util.c https://github.com/YottaDB/YottaDB/pull/190#discussion_r176951341:

boolean_t   status;

gd_id_ptr_t tmp_fileid;

  if (!filename)
      return FALSE;
  assert(fileid && !*fileid);
  tmp_fileid = (gd_id_ptr_t)malloc(SIZEOF(gd_id));
  • status = (SS_NORMAL == filename_to_id(tmp_fileid, filename->address));
  • *fileid = (xc_fileid_ptr_t)tmp_fileid;
  • return status;
  • actstatus = filename_to_id(tmp_fileid, filename->address);
  • status = (SS_NORMAL == actstatus);
  • if (status)
  • *fileid = (ydb_fileid_ptr_t)tmp_fileid;
  • else
  • { / There was an error /
  • free(tmp_fileid);
  • *fileid = NULL;
  • }
  • return status ? YDB_OK : actstatus;

Here is the description of ydb_file_name_to_id() which is a wrapper for gtm_filename_to_id():

ydb_file_name_to_id() returns YDB_OK /or an error return code./

We can not do that hilighted part because the original version of gtm_filename_to_id() does not do return codes. It returns 1 if it was successful and 0 if it wasn't but does not return any information. Any attempt to rework this routine to return error information is equivalent to returning "TRUE" when it isn't. I need to revert this routine and we can only return success/failure. The spec needs to change for ydb_file_name_to_id() because of this.

[KSB] How about if we define a constant YDB_NOTOK and have the function return YDB_OK or YDB_NOTOK?

– Bhaskar

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/YottaDB/YottaDB/pull/190#discussion_r176951341, or mute the thread https://github.com/notifications/unsubscribe-auth/AcfILQd7RBUNOkPbT3qducTw8QxvvAjdks5th_HFgaJpZM4S5F4U.

-- YottaDB - Rock solid. Lightning fast. Secure. Pick any three.