Hello, please see updated version of the patch.
On 09/16/2015 11:58 AM, Jakub Hrozek wrote:
On Mon, Sep 14, 2015 at 07:53:06PM +0200, Pavel Reichl wrote:
+static uint8_t *nsupdate_convert_address(struct sockaddr_storage *add_address) +{
- uint8_t *addr;
- switch(add_address->ss_family) {
- case AF_INET:
addr = (uint8_t *) &((struct sockaddr_in *) add_address)->sin_addr;
break;
- case AF_INET6:
addr = (uint8_t *) &((struct sockaddr_in6 *) add_address)->sin6_addr;
break;
- default:
DEBUG(SSSDBG_CRIT_FAILURE, "Unknown address family\n");
addr = NULL;
break;
- }
- return addr;
+}
Not directly related to this patch, but this going back and forth between sockaddr_storage and uint8_t pointer made me wonder if we should change resolv_get_string_ptr_address to accept sockaddr storage instead of these hacks?
I filled the https://fedorahosted.org/sssd/ticket/2793
+static char *nsupdate_msg_add_ptr(char *update_msg,
struct sockaddr_storage *address,
const char *hostname,
int ttl,
uint8_t remove_af,
{bool delete)
struct sss_iface_addr *new_record, *old_record; char *strptr; uint8_t *addr;
DLIST_FOR_EACH(old_record, old_addresses) {
switch(old_record->addr->ss_family) {
case AF_INET:
if (!(remove_af & DYNDNS_REMOVE_A)) {
continue;
}
addr = (uint8_t *) &((struct sockaddr_in *) old_record->addr)->sin_addr;
break;
case AF_INET6:
if (!(remove_af & DYNDNS_REMOVE_AAAA)) {
continue;
}
addr = (uint8_t *) &((struct sockaddr_in6 *) old_record->addr)->sin6_addr;
break;
default:
DEBUG(SSSDBG_CRIT_FAILURE, "Unknown address family\n");
return NULL;
}
- if (delete
&& !nsupdate_should_remove_addr(address->ss_family, remove_af)) {
This seems a bit strange to me, because if we hit this condition, then the whole request is aborted. Shouldn't we just skip this address instead?
That was definitely bug, thanks for noticing it. However I didn't see any other way how to fix this unless to move the logic to sdap layer.
be_nsupdate_create_ptr_msg(TALLOC_CTX *mem_ctx, const char *realm, const char *servername, const char *hostname, const unsigned int ttl, uint8_t remove_af,
struct sss_iface_addr *addresses,
struct sss_iface_addr *old_addresses,
struct sockaddr_storage *address,
bool del_phase,
I don't think the option should contain 'phase' in its name, the fact that we have some phases is a detail from the sdap layer.
char **_update_msg);
I renamed it to delete, but I'm not sure that's exactly what you had in mind.