-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
INTERNAL: Fix compile errors with zookeeper in redhat8 #283
Conversation
@namsic 리뷰 바랍니다. |
libmemcached/arcus.cc
Outdated
if (strlen(serverlist) >= ARCUS_MAX_PROXY_FILE_LENGTH) { | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이런 형태로 Werror를 회피하는 것보다, snprintf
를 사용하는 구현이 나을 것으로 생각합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 do_arcus_zk_update_cachelist_by_string
함수에 주어지는
serverlist의 크기는 9728로 버퍼의 크기인 10240(ARCUS_MAX_PROXY_FILE_LENGTH
)보다 작고,
snprintf
로 인해 문자열이 잘리게 되어 잘못된 문자열이 저장되는 현상은 없다고 생각되지만
확실하게 보장하는 편이 낫다고 생각하여 이전 PR(strncpy)과 동일하게 수정했습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이전 PR에서는 함수가 에러를 반환할 수 있기 때문에 strncpy()
호출 전에 src 길이를 확인하고 에러를 반환하는 것은 나름대로 합리적인 동작으로 보입니다,
그런데 현재 PR에서는 반환값이 없으므로 단순히 수행 중단하도록 변경했습니다.
=> 이 때 문자열 길이가 길면 겉으로 드러나는 문제는 없으면서 의도한대로 동작하지 않을 것 같은데, 이렇게 동작해도 무방한가요?
그리고, 현재 구현은 메모리 누수 가능성도 있어 보입니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
메모리 누수 가능성은 확인했습니다.
return 부분에 free를 추가하거나 로직 순서를 변경하는 것이 나을 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그런데 현재 PR에서는 반환값이 없으므로 단순히 수행 중단하도록 변경했습니다.
=> 이 때 문자열 길이가 길면 겉으로 드러나는 문제는 없으면서 의도한대로 동작하지 않을 것 같은데, 이렇게 동작해도 무방한가요?
결국은 서버리스트 update를 하지 않게 되므로 무방하지는 않습니다. 하지만 sprintf
를 활용한다면 더 위험해지지 않을까 생각됩니다. sprintf의 경우 끝에 \0
을 추가하여 컴파일 에러를 보장해주는 것은 맞지만 잘못된 문자열이 만들어질 수 있습니다. 이 상황에서 서버리스트를 update하여 distributio hash를 변경(run_distribution
)한다면, 잘못된 동작을 하게 됩니다.
해당 함수는 내부 함수로 사용하는 인자에서 길이가 너무 큰 문자열을 보내는 경우가 없으며, 실제로 서버리스트의 변화는 초기의 arcus_connect
수행에서 arcus_zk_manager
라는 별도의 쓰레드를 두어 감시하고 있는 것으로 보입니다. 그렇기 때문에 별다른 에러 로직을 두지 않고, 바로 리턴을 시킨게 아닐까 싶습니다.
@@ -556,7 +556,7 @@ static inline int do_arcus_cluster_validation_check(memcached_st *mc, arcus_st * | |||
static inline int do_add_client_info(arcus_st *arcus) | |||
{ | |||
int result; | |||
char path[250]; | |||
char path[512]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
client_list znode의 최대 길이는 400 이하인 것으로 보이므로, 512 정도면 크게 문제는 없을 것으로 생각합니다.
@@ -30,7 +30,7 @@ struct arcus_zk_st | |||
clientid_t myid; | |||
char ensemble_list[1024]; | |||
char svc_code[256]; | |||
char path[256]; | |||
char path[512]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 path 버퍼 크기가 512로 변경된 이유는 무엇인가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
truncated writing error와 동일하며, 위의 sprintf
를 활용하여 문자열 구성할 때 svc_code 변수의 크기와 path 변수 크기와 동일하여 담는 문자열이 path 변수 크기를 초과할 수 있기에 생기는 에러입니다. 에러 내용으로보아 512byte까지는 필요 없다고 생각이 드나 2의 지수로 설정하는 편이 깔끔해 보였고, 넉넉한 공간 확보하는 편이 좋다고 생각이 들어 512byte로 수정했습니다.
libmemcached/arcus.cc:530:5: error: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 233 [-Werror=format-truncation=]
"%s/%s", ARCUS_REPL_ZK_CACHE_LIST, arcus->zk.svc_code);
^~~~~~~
libmemcached/arcus.cc:529:11: note: ‘snprintf’ output between 24 and 279 bytes into a destination of size 256
snprintf(arcus->zk.path, sizeof(arcus->zk.path),
~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"%s/%s", ARCUS_REPL_ZK_CACHE_LIST, arcus->zk.svc_code);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libmemcached/arcus.cc:541:5: error: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 238 [-Werror=format-truncation=]
"%s/%s", ARCUS_ZK_CACHE_LIST, arcus->zk.svc_code);
^~~~~~~
libmemcached/arcus.cc:540:11: note: ‘snprintf’ output between 19 and 274 bytes into a destination of size 256
snprintf(arcus->zk.path, sizeof(arcus->zk.path),
~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"%s/%s", ARCUS_ZK_CACHE_LIST, arcus->zk.svc_code);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libmemcached/arcus.cc: In function ‘arcus_return_t arcus_connect(memcached_st*, const char*, const char*)’:
libmemcached/arcus.cc:131:14: error: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 238 [-Werror=format-truncation=]
"%s/%s", ARCUS_ZK_CACHE_LIST, arcus->zk.svc_code);
^~~~~~~
libmemcached/arcus.cc:130:13: note: ‘snprintf’ output between 19 and 274 bytes into a destination of size 256
snprintf(arcus->zk.path, sizeof(arcus->zk.path),
~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"%s/%s", ARCUS_ZK_CACHE_LIST, arcus->zk.svc_code);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libmemcached/arcus.cc:530:5: error: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 233 [-Werror=format-truncation=]
"%s/%s", ARCUS_REPL_ZK_CACHE_LIST, arcus->zk.svc_code);
^~~~~~~
libmemcached/arcus.cc:529:11: note: ‘snprintf’ output between 24 and 279 bytes into a destination of size 256
snprintf(arcus->zk.path, sizeof(arcus->zk.path),
~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"%s/%s", ARCUS_REPL_ZK_CACHE_LIST, arcus->zk.svc_code);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libmemcached/arcus.cc:541:5: error: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 238 [-Werror=format-truncation=]
"%s/%s", ARCUS_ZK_CACHE_LIST, arcus->zk.svc_code);
^~~~~~~
libmemcached/arcus.cc:540:11: note: ‘snprintf’ output between 19 and 274 bytes into a destination of size 256
snprintf(arcus->zk.path, sizeof(arcus->zk.path),
~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"%s/%s", ARCUS_ZK_CACHE_LIST, arcus->zk.svc_code);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libmemcached/arcus.cc: In function ‘arcus_return_t arcus_pool_connect(memcached_pool_st*, const char*, const char*)’:
libmemcached/arcus.cc:131:14: error: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 238 [-Werror=format-truncation=]
"%s/%s", ARCUS_ZK_CACHE_LIST, arcus->zk.svc_code);
^~~~~~~
libmemcached/arcus.cc:130:13: note: ‘snprintf’ output between 19 and 274 bytes into a destination of size 256
snprintf(arcus->zk.path, sizeof(arcus->zk.path),
~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"%s/%s", ARCUS_ZK_CACHE_LIST, arcus->zk.svc_code);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libmemcached/arcus.cc:530:5: error: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 233 [-Werror=format-truncation=]
"%s/%s", ARCUS_REPL_ZK_CACHE_LIST, arcus->zk.svc_code);
^~~~~~~
libmemcached/arcus.cc:529:11: note: ‘snprintf’ output between 24 and 279 bytes into a destination of size 256
snprintf(arcus->zk.path, sizeof(arcus->zk.path),
~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"%s/%s", ARCUS_REPL_ZK_CACHE_LIST, arcus->zk.svc_code);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libmemcached/arcus.cc:541:5: error: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 238 [-Werror=format-truncation=]
"%s/%s", ARCUS_ZK_CACHE_LIST, arcus->zk.svc_code);
^~~~~~~
libmemcached/arcus.cc:540:11: note: ‘snprintf’ output between 19 and 274 bytes into a destination of size 256
snprintf(arcus->zk.path, sizeof(arcus->zk.path),
~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"%s/%s", ARCUS_ZK_CACHE_LIST, arcus->zk.svc_code);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libmemcached/arcus.cc: In function ‘arcus_return_t arcus_proxy_create(memcached_st*, const char*, const char*)’:
libmemcached/arcus.cc:131:14: error: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 238 [-Werror=format-truncation=]
"%s/%s", ARCUS_ZK_CACHE_LIST, arcus->zk.svc_code);
^~~~~~~
libmemcached/arcus.cc:130:13: note: ‘snprintf’ output between 19 and 274 bytes into a destination of size 256
snprintf(arcus->zk.path, sizeof(arcus->zk.path),
~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"%s/%s", ARCUS_ZK_CACHE_LIST, arcus->zk.svc_code);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libmemcached/arcus.cc:530:5: error: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 233 [-Werror=format-truncation=]
"%s/%s", ARCUS_REPL_ZK_CACHE_LIST, arcus->zk.svc_code);
^~~~~~~
libmemcached/arcus.cc:529:11: note: ‘snprintf’ output between 24 and 279 bytes into a destination of size 256
snprintf(arcus->zk.path, sizeof(arcus->zk.path),
~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"%s/%s", ARCUS_REPL_ZK_CACHE_LIST, arcus->zk.svc_code);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libmemcached/arcus.cc:541:5: error: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 238 [-Werror=format-truncation=]
"%s/%s", ARCUS_ZK_CACHE_LIST, arcus->zk.svc_code);
^~~~~~~
libmemcached/arcus.cc:540:11: note: ‘snprintf’ output between 19 and 274 bytes into a destination of size 256
snprintf(arcus->zk.path, sizeof(arcus->zk.path),
~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"%s/%s", ARCUS_ZK_CACHE_LIST, arcus->zk.svc_code);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libmemcached/arcus.cc: In function ‘arcus_return_t arcus_proxy_connect(memcached_st*, memcached_pool_st*, memcached_st*)’:
libmemcached/arcus.cc:131:14: error: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 238 [-Werror=format-truncation=]
"%s/%s", ARCUS_ZK_CACHE_LIST, arcus->zk.svc_code);
^~~~~~~
libmemcached/arcus.cc:130:13: note: ‘snprintf’ output between 19 and 274 bytes into a destination of size 256
snprintf(arcus->zk.path, sizeof(arcus->zk.path),
~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"%s/%s", ARCUS_ZK_CACHE_LIST, arcus->zk.svc_code);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
with-zk-integration
옵션을 넣고 compile 시, redhat8 계열에서 일어나는 컴파일 에러 수정입니다.에러 원인 및 처리
sprintf
수행 시 output 버퍼의 크기가 (input 버퍼의 크기 + 기타 문자열 크기) 보다 작음.strnpcy
수행 시 input 문자열의 크기가 output 버퍼보다 클 수 있음