[CBRD-25978] refactoring to system parameters #6090
Conversation
…toring-to-system_parameters
…toring-to-system_parameters
src/base/system_parameter.c
Outdated
| @@ -12828,7 +12763,7 @@ sysprm_set_sysprm_value_from_parameter (SYSPRM_VALUE * prm_value, SYSPRM_PARAM * | |||
| case PRM_STRING: | |||
| if (PRM_GET_STRING (prm->value) != NULL) | |||
| { | |||
| prm_value->str = strdup (PRM_GET_STRING (prm->value)); | |||
| prm_value->str = strdup (PRM_GET_STRING (prm->value)); // TODO: prm_value == prm->value 인 경우는 leak 발생 위험이 있음. | |||
There was a problem hiding this comment.
[질문] 어떻게 leak 이 발생는지 설명 부탁드립니다.
There was a problem hiding this comment.
-
prm->value와 prm_value는 같아서는 않됩니다.
같다면 자기 자신의 데이터로 자기 자신에게 작업을 하는 문제가 발생합니다. -
prm_value->str 에 이미 메모리 할당되어 있는 상태인 경우 해당 메모리가 leak이 됩니다.
| { | ||
| int group_code; | ||
| int dim; | ||
| int *integer_list = NULL; | ||
|
|
||
| group_code = PRM_GET_INT (fault_injection_test_prm->value); | ||
|
|
||
| if (PRM_IS_ALLOCATED (*fault_injection_ids_prm->dynamic_flag)) | ||
| if (PRM_IS_ALLOCATED (fault_injection_ids_prm)) |
There was a problem hiding this comment.
가독성을 위해 PRM_IS_ALLOCATED 보다는 PRM_IS_DYNAMIC 혹은 PRM_IS_DYNAMIC_ALLOCATED가 어떨지요.
| if (msgcat_System[i].msg_catd == NULL) | ||
| { | ||
| // TODO: Need to decide whether to return immediately when an error occurs. | ||
| rc = ER_FAILED; |
There was a problem hiding this comment.
| rc = ER_FAILED; | |
| rc = ER_FAILED; | |
| break; |
위와 같이 처리가 가능할까요?
There was a problem hiding this comment.
이 부분은 msgcat_init() 함수가 기존과 동일하게 동작하도록 유지하면서 반복문을 따로 한번 도는 비효율을 막은 것입니다.
기존 코드의 로직을 유지하려다 보니, 에러가 있어도 계속 루프를 도는 것입니다.
강민님, 의견은 일리가 있어 보이고 저도 그렇게 생각합니다만 기존 코드에서 그렇게 처리한 이유를 명확히 파악할 수 없어(시간 관계상) 일단 동일한 결과가 되도록 유지 했습니다.
대신 TODO 문구를 남겨 둬서 후일을 기약하는 것으로...
| @@ -116,6 +116,9 @@ static const char sysprm_ha_conf_file_name[] = "cubrid_ha.conf"; | |||
| * System variable names | |||
| */ | |||
|
|
|||
| // *INDENT-OFF* //======================================================================== | |||
| #if 1 // [START] To fold source code for readability in development tools | |||
There was a problem hiding this comment.
folding을 위한 목적이라면 다음과 같은 방법은 어떠신가요?
- vscode editor: #region folding for VS Code 플러그인 설치
- vim editor: vim-fold 플러그인 설치 후 fold-marker 설정
//#region [message]
...
//#endregion| m_loaded[m_used].conf_path = strdup (conf_path); | ||
| m_loaded[m_used].db_name = db_name ? strdup (db_name) : NULL; |
There was a problem hiding this comment.
strdup 가 실패했을 경우에 대한 처리는 필요 없을까요?
There was a problem hiding this comment.
m_loaded에 저장해 둔 정보가 매우 중요하게 사용되지는 않는 듯 합니다.
즉 동작에 영향을 미치지 않습니다.
다만 paramdump할 때 상단에 보여지는 파일명일 뿐이라서,
strdup()이 실패할 일이 없겠지만, 만에 하나 실패하더라도 무시하고 진행해도 무방할 것으로 판단됩니다.
기존 코드도 그렇게 되어 있었구요^^
| prm_section_cmp (const char *key1, const char *key2, int key_len, bool ignore_case) | ||
| { | ||
| const char *s1 = strchr (key1, ':'); | ||
| int len = (int) ((s1) ? CAST_STRLEN (s1 - key1) : strlen (key1)); |
There was a problem hiding this comment.
| int len = (int) ((s1) ? CAST_STRLEN (s1 - key1) : strlen (key1)); | |
| int len = (s1) ? CAST_STRLEN (s1 - key1) : strlen (key1); |
이미 캐스팅되어 있어서 추가적인 캐스팅이 필요하지 않을을 거 같습니다.
There was a problem hiding this comment.
strlen()의 결과도 casting 대상이 되어야 합니다. size_t --> int
| on_client = true; | ||
| #endif | ||
| sec_p = strchr (section, ':'); | ||
| sec_key_len = (int) ((sec_p) ? CAST_STRLEN (sec_p - section) : strlen (section)); |
There was a problem hiding this comment.
| sec_key_len = (int) ((sec_p) ? CAST_STRLEN (sec_p - section) : strlen (section)); | |
| sec_key_len = (int) ((sec_p) ? CAST_STRLEN (sec_p - section) : strlen (section)); |
| sec_key_len = (int) ((sec_p) ? CAST_STRLEN (sec_p - section) : strlen (section)); | |
| sec_key_len = (sec_p) ? CAST_STRLEN (sec_p - section) : strlen (section); |
이미 캐스팅되어 있어서 추가적인 캐스팅이 필요하지 않을을 거 같습니다.
There was a problem hiding this comment.
strlen()의 결과도 casting 대상이 되어야 합니다.
http://jira.cubrid.org/browse/CBRD-25978
Purpose
Implementation
sysprm_set_force(), sysprm_set_to_default()
sysprm_check_range(), sysprm_get_range()
Remarks
N/A