Unverified Commit 0b643e93 authored by Qu Chen's avatar Qu Chen Committed by GitHub
Browse files

Cleanup: createAOFClient uses createClient to avoid overlooked mismatches (#9338)

AOF fake client creation (createAOFClient) was doing similar work as createClient,
with some minor differences, most of which unintended, this was dangerous and
meant that many changes to createClient should have always been reflected to aof.c

This cleanup changes createAOFClient to call createClient with NULL, like we
do in module.c and elsewhere.
parent d3356bf6
...@@ -42,6 +42,7 @@ ...@@ -42,6 +42,7 @@
void aofUpdateCurrentSize(void); void aofUpdateCurrentSize(void);
void aofClosePipes(void); void aofClosePipes(void);
void freeClientArgv(client *c);
/* ---------------------------------------------------------------------------- /* ----------------------------------------------------------------------------
* AOF rewrite buffer implementation. * AOF rewrite buffer implementation.
...@@ -639,21 +640,9 @@ void feedAppendOnlyFile(int dictid, robj **argv, int argc) { ...@@ -639,21 +640,9 @@ void feedAppendOnlyFile(int dictid, robj **argv, int argc) {
/* In Redis commands are always executed in the context of a client, so in /* In Redis commands are always executed in the context of a client, so in
* order to load the append only file we need to create a fake client. */ * order to load the append only file we need to create a fake client. */
struct client *createAOFClient(void) { struct client *createAOFClient(void) {
struct client *c = zmalloc(sizeof(*c)); struct client *c = createClient(NULL);
selectDb(c,0);
c->id = CLIENT_ID_AOF; /* So modules can identify it's the AOF client. */ c->id = CLIENT_ID_AOF; /* So modules can identify it's the AOF client. */
c->conn = NULL;
c->name = NULL;
c->querybuf = sdsempty();
c->querybuf_peak = 0;
c->argc = 0;
c->argv = NULL;
c->original_argc = 0;
c->original_argv = NULL;
c->argv_len_sum = 0;
c->bufpos = 0;
c->buf_usable_size = zmalloc_usable_size(c)-offsetof(client,buf);
/* /*
* The AOF client should never be blocked (unlike master * The AOF client should never be blocked (unlike master
...@@ -666,42 +655,12 @@ struct client *createAOFClient(void) { ...@@ -666,42 +655,12 @@ struct client *createAOFClient(void) {
*/ */
c->flags = CLIENT_DENY_BLOCKING; c->flags = CLIENT_DENY_BLOCKING;
c->btype = BLOCKED_NONE;
/* We set the fake client as a slave waiting for the synchronization /* We set the fake client as a slave waiting for the synchronization
* so that Redis will not try to send replies to this client. */ * so that Redis will not try to send replies to this client. */
c->replstate = SLAVE_STATE_WAIT_BGSAVE_START; c->replstate = SLAVE_STATE_WAIT_BGSAVE_START;
c->reply = listCreate();
c->reply_bytes = 0;
c->obuf_soft_limit_reached_time = 0;
c->watched_keys = listCreate();
c->peerid = NULL;
c->sockname = NULL;
c->resp = 2;
c->user = NULL;
listSetFreeMethod(c->reply,freeClientReplyValue);
listSetDupMethod(c->reply,dupClientReplyValue);
initClientMultiState(c);
return c; return c;
} }
void freeFakeClientArgv(struct client *c) {
int j;
for (j = 0; j < c->argc; j++)
decrRefCount(c->argv[j]);
zfree(c->argv);
c->argv_len_sum = 0;
}
void freeFakeClient(struct client *c) {
sdsfree(c->querybuf);
listRelease(c->reply);
listRelease(c->watched_keys);
freeClientMultiState(c);
freeClientOriginalArgv(c);
zfree(c);
}
/* Replay the append log file. On success AOF_OK is returned, /* Replay the append log file. On success AOF_OK is returned,
* otherwise, one of the following is returned: * otherwise, one of the following is returned:
* AOF_OPEN_ERR: Failed to open the AOF file. * AOF_OPEN_ERR: Failed to open the AOF file.
...@@ -806,7 +765,7 @@ int loadAppendOnlyFile(char *filename) { ...@@ -806,7 +765,7 @@ int loadAppendOnlyFile(char *filename) {
char *readres = fgets(buf,sizeof(buf),fp); char *readres = fgets(buf,sizeof(buf),fp);
if (readres == NULL || buf[0] != '$') { if (readres == NULL || buf[0] != '$') {
fakeClient->argc = j; /* Free up to j-1. */ fakeClient->argc = j; /* Free up to j-1. */
freeFakeClientArgv(fakeClient); freeClientArgv(fakeClient);
if (readres == NULL) if (readres == NULL)
goto readerr; goto readerr;
else else
...@@ -819,7 +778,7 @@ int loadAppendOnlyFile(char *filename) { ...@@ -819,7 +778,7 @@ int loadAppendOnlyFile(char *filename) {
if (len && fread(argsds,len,1,fp) == 0) { if (len && fread(argsds,len,1,fp) == 0) {
sdsfree(argsds); sdsfree(argsds);
fakeClient->argc = j; /* Free up to j-1. */ fakeClient->argc = j; /* Free up to j-1. */
freeFakeClientArgv(fakeClient); freeClientArgv(fakeClient);
goto readerr; goto readerr;
} }
argv[j] = createObject(OBJ_STRING,argsds); argv[j] = createObject(OBJ_STRING,argsds);
...@@ -827,7 +786,7 @@ int loadAppendOnlyFile(char *filename) { ...@@ -827,7 +786,7 @@ int loadAppendOnlyFile(char *filename) {
/* Discard CRLF. */ /* Discard CRLF. */
if (fread(buf,2,1,fp) == 0) { if (fread(buf,2,1,fp) == 0) {
fakeClient->argc = j+1; /* Free up to j. */ fakeClient->argc = j+1; /* Free up to j. */
freeFakeClientArgv(fakeClient); freeClientArgv(fakeClient);
goto readerr; goto readerr;
} }
} }
...@@ -838,7 +797,7 @@ int loadAppendOnlyFile(char *filename) { ...@@ -838,7 +797,7 @@ int loadAppendOnlyFile(char *filename) {
serverLog(LL_WARNING, serverLog(LL_WARNING,
"Unknown command '%s' reading the append only file", "Unknown command '%s' reading the append only file",
(char*)argv[0]->ptr); (char*)argv[0]->ptr);
freeFakeClientArgv(fakeClient); freeClientArgv(fakeClient);
ret = AOF_FAILED; ret = AOF_FAILED;
goto cleanup; goto cleanup;
} }
...@@ -864,7 +823,9 @@ int loadAppendOnlyFile(char *filename) { ...@@ -864,7 +823,9 @@ int loadAppendOnlyFile(char *filename) {
/* Clean up. Command code may have changed argv/argc so we use the /* Clean up. Command code may have changed argv/argc so we use the
* argv/argc of the client instead of the local variables. */ * argv/argc of the client instead of the local variables. */
freeFakeClientArgv(fakeClient); freeClientArgv(fakeClient);
zfree(fakeClient->argv);
fakeClient->argv = NULL;
fakeClient->cmd = NULL; fakeClient->cmd = NULL;
if (server.aof_load_truncated) valid_up_to = ftello(fp); if (server.aof_load_truncated) valid_up_to = ftello(fp);
if (server.key_load_delay) if (server.key_load_delay)
...@@ -932,7 +893,7 @@ fmterr: /* Format error. */ ...@@ -932,7 +893,7 @@ fmterr: /* Format error. */
/* fall through to cleanup. */ /* fall through to cleanup. */
cleanup: cleanup:
if (fakeClient) freeFakeClient(fakeClient); /* avoid valgrind warning */ if (fakeClient) freeClient(fakeClient); /* avoid valgrind warning */
fclose(fp); fclose(fp);
stopLoading(ret == AOF_OK); stopLoading(ret == AOF_OK);
return ret; return ret;
......
...@@ -1184,7 +1184,7 @@ void freeClientOriginalArgv(client *c) { ...@@ -1184,7 +1184,7 @@ void freeClientOriginalArgv(client *c) {
c->original_argc = 0; c->original_argc = 0;
} }
static void freeClientArgv(client *c) { void freeClientArgv(client *c) {
int j; int j;
for (j = 0; j < c->argc; j++) for (j = 0; j < c->argc; j++)
decrRefCount(c->argv[j]); decrRefCount(c->argv[j]);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment