client: check for COMMAND_RETURN_CLOSE
Don't close the client within client_process_line(), return COMMAND_RETURN_CLOSE instead. This is the signal for the caller chain to actually close it. This makes dealing with the client pointer a lot safer, since the caller always knows whether it is still valid.
This commit is contained in:
		
							
								
								
									
										29
									
								
								src/client.c
									
									
									
									
									
								
							
							
						
						
									
										29
									
								
								src/client.c
									
									
									
									
									
								
							| @@ -343,10 +343,8 @@ static int client_process_line(struct client *client) | |||||||
| 			      "list returned %i\n", client->num, ret); | 			      "list returned %i\n", client->num, ret); | ||||||
|  |  | ||||||
| 			if (ret == COMMAND_RETURN_CLOSE || | 			if (ret == COMMAND_RETURN_CLOSE || | ||||||
| 			    client_is_expired(client)) { | 			    client_is_expired(client)) | ||||||
| 				client_close(client); |  | ||||||
| 				return COMMAND_RETURN_CLOSE; | 				return COMMAND_RETURN_CLOSE; | ||||||
| 			} |  | ||||||
|  |  | ||||||
| 			if (ret == 0) | 			if (ret == 0) | ||||||
| 				command_success(client); | 				command_success(client); | ||||||
| @@ -368,8 +366,7 @@ static int client_process_line(struct client *client) | |||||||
| 				      (unsigned long)client->cmd_list_size, | 				      (unsigned long)client->cmd_list_size, | ||||||
| 				      (unsigned long) | 				      (unsigned long) | ||||||
| 				      client_max_command_list_size); | 				      client_max_command_list_size); | ||||||
| 				client_close(client); | 				return COMMAND_RETURN_CLOSE; | ||||||
| 				ret = COMMAND_RETURN_CLOSE; |  | ||||||
| 			} else | 			} else | ||||||
| 				new_cmd_list_ptr(client, line, len); | 				new_cmd_list_ptr(client, line, len); | ||||||
| 		} | 		} | ||||||
| @@ -388,10 +385,8 @@ static int client_process_line(struct client *client) | |||||||
| 			      client->num, ret); | 			      client->num, ret); | ||||||
|  |  | ||||||
| 			if (ret == COMMAND_RETURN_CLOSE || | 			if (ret == COMMAND_RETURN_CLOSE || | ||||||
| 			    client_is_expired(client)) { | 			    client_is_expired(client)) | ||||||
| 				client_close(client); |  | ||||||
| 				return COMMAND_RETURN_CLOSE; | 				return COMMAND_RETURN_CLOSE; | ||||||
| 			} |  | ||||||
|  |  | ||||||
| 			if (ret == 0) | 			if (ret == 0) | ||||||
| 				command_success(client); | 				command_success(client); | ||||||
| @@ -422,16 +417,14 @@ static int client_input_received(struct client *client, int bytesRead) | |||||||
| 			if (ret == COMMAND_RETURN_KILL || | 			if (ret == COMMAND_RETURN_KILL || | ||||||
| 			    ret == COMMAND_RETURN_CLOSE) | 			    ret == COMMAND_RETURN_CLOSE) | ||||||
| 				return ret; | 				return ret; | ||||||
| 			if (client_is_expired(client)) | 			assert(!client_is_expired(client)); | ||||||
| 				return ret; |  | ||||||
| 			client->bufferPos = client->bufferLength; | 			client->bufferPos = client->bufferLength; | ||||||
| 		} | 		} | ||||||
| 		if (client->bufferLength == CLIENT_MAX_BUFFER_LENGTH) { | 		if (client->bufferLength == CLIENT_MAX_BUFFER_LENGTH) { | ||||||
| 			if (client->bufferPos == 0) { | 			if (client->bufferPos == 0) { | ||||||
| 				ERROR("client %i: buffer overflow\n", | 				ERROR("client %i: buffer overflow\n", | ||||||
| 				      client->num); | 				      client->num); | ||||||
| 				client_close(client); | 				return COMMAND_RETURN_CLOSE; | ||||||
| 				return 1; |  | ||||||
| 			} | 			} | ||||||
| 			if (client->cmd_list_OK >= 0 && | 			if (client->cmd_list_OK >= 0 && | ||||||
| 			    client->cmd_list && | 			    client->cmd_list && | ||||||
| @@ -461,7 +454,7 @@ static int client_read(struct client *client) | |||||||
| 	if (bytesRead > 0) | 	if (bytesRead > 0) | ||||||
| 		return client_input_received(client, bytesRead); | 		return client_input_received(client, bytesRead); | ||||||
| 	else if (bytesRead == 0 || (bytesRead < 0 && errno != EINTR)) { | 	else if (bytesRead == 0 || (bytesRead < 0 && errno != EINTR)) { | ||||||
| 		client_close(client); | 		return COMMAND_RETURN_CLOSE; | ||||||
| 	} else | 	} else | ||||||
| 		return 0; | 		return 0; | ||||||
|  |  | ||||||
| @@ -532,10 +525,16 @@ int client_manager_io(void) | |||||||
|  |  | ||||||
| 	list_for_each_entry_safe(client, n, &clients, siblings) { | 	list_for_each_entry_safe(client, n, &clients, siblings) { | ||||||
| 		if (FD_ISSET(client->fd, &rfds)) { | 		if (FD_ISSET(client->fd, &rfds)) { | ||||||
| 			if (COMMAND_RETURN_KILL == | 			ret = client_read(client); | ||||||
| 			    client_read(client)) { | 			if (ret == COMMAND_RETURN_KILL) | ||||||
| 				return COMMAND_RETURN_KILL; | 				return COMMAND_RETURN_KILL; | ||||||
|  | 			if (ret == COMMAND_RETURN_CLOSE) { | ||||||
|  | 				client_close(client); | ||||||
|  | 				continue; | ||||||
| 			} | 			} | ||||||
|  |  | ||||||
|  | 			assert(!client_is_expired(client)); | ||||||
|  |  | ||||||
| 			client->lastTime = time(NULL); | 			client->lastTime = time(NULL); | ||||||
| 		} | 		} | ||||||
| 		if (!client_is_expired(client) && | 		if (!client_is_expired(client) && | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Max Kellermann
					Max Kellermann