[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [tyndur-devel] [PATCH] dns_request loest nun Namen in mehrere IP-Adressen auf.



On Wed, Nov 25, 2009 at 07:07:59PM +0100, Roman Muentener wrote:
> + in dns.c: dns_request_result; dns_free_result;
> * in dns.c: cache handling angepasst
> * in lostio_if.c: Interface angepasst, damit die neue Aufloesung klappt.

Die Beschreibung ist viel zu detailliert, damit kann niemand mehr was
anfangen, wenn er es in einem Jahr liet. Besser sowas:

+ tcpip: DNS-Abfragen geben jetzt alle IP-Adressen zurueck

> + getip: Ein Programm dass einen Namen in mehrere IP-Adressen aufloest
>   und in der Shell ausgibt
> 
> Signed-off-by: Roman Muentener <fmnssun@xxxxxxxxx>

Schreibst du dich tatsächlich mit ue oder ist das nur wegen ASCII? git
kann mit UTF-8 ganz gut umgehen.

> ---
>  src/modules/c/getip/Makefile.all |    7 +++
>  src/modules/c/getip/main.c       |   80 ++++++++++++++++++++++++++++++++++++++
>  src/modules/tcpip/dns.c          |   68 +++++++++++++++++++++++--------
>  src/modules/tcpip/include/dns.h  |    9 ++++-
>  src/modules/tcpip/lostio_if.c    |   35 ++++++++++++++---
>  5 files changed, 174 insertions(+), 25 deletions(-)
>  create mode 100644 src/modules/c/getip/Makefile.all
>  create mode 100644 src/modules/c/getip/main.c
> 
> diff --git a/src/modules/c/getip/Makefile.all b/src/modules/c/getip/Makefile.all
> new file mode 100644
> index 0000000..59d796e
> --- /dev/null
> +++ b/src/modules/c/getip/Makefile.all
> @@ -0,0 +1,7 @@
> +shopt -s extglob
> +source $LOST_BUILDMK_ROOT/config.sh
> +
> +echo "LD   $1/apps/getip"
> +$LOST_TOOLS_LD -ogetip -Ttext=0x40000000 *.o --start-group $2 --end-group
> +
> +mv getip $1/apps/

Wie gesagt, idealerweise wird getip durch cat tcpip:/dns/foo
überflüssig. Ich werden trotzdem ein paar allgemein gültige Anmerkungen
einschieben. Übrigens wäre sonst auch zu überlegen, ob man das nicht als
zusätzlichen Befehl in die Shell integriert, um Platz zu sparen. Einfach
genug wäre es.

> diff --git a/src/modules/c/getip/main.c b/src/modules/c/getip/main.c
> new file mode 100644
> index 0000000..4a1f411
> --- /dev/null
> +++ b/src/modules/c/getip/main.c
> @@ -0,0 +1,80 @@
> +/*  
> + * Copyright (c) 2007 The tyndur Project. All rights reserved.

Wir haben 2009. ;-) Der zweite Fehler beim Kopieren von existierendem
älteren Code ist, dass er häufig Leerzeichen am Zeilenende enthält, das
ist auch in diesem Kommentar so.

> + *
> + * This code is derived from software contributed to the tyndur Project
> + * by Roman Muentener.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in the
> + *    documentation and/or other materials provided with the distribution.
> + * 3. All advertising materials mentioning features or use of this software
> + *    must display the following acknowledgement:
> + *     This product includes software developed by the tyndur Project
> + *     and its contributors.
> + * 4. Neither the name of the tyndur Project nor the names of its
> + *    contributors may be used to endorse or promote products derived
> + *    from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS 
> + * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
> + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
> + * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR 
> + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, 
> + * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, 
> + * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; 
> + * OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, 
> + * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR 
> + * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF 
> + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +
> +#include "stdlib.h"
> +#include "string.h"
> +#include "netdb.h"
> +#include "stdio.h"
> +#include "arpa/inet.h"
> +#include "errno.h"
> +#include "network.h"

Sind alles Systemheader, also #include <stdlib.h> usw.

> +
> +
> +int main(int argc, char* argv[])
> +{
> +    printf("getip\n-----\n\n");
> +    if(argc != 2) {
> +        printf("Incorrect usage!\n");
> +        printf("Correct usage:\tgetip <hostname>\n");

Sofern ein Programm nicht mehrere Sprachen unterstützt, sind alle
Meldungen deutsch.

> +        return 1;
> +    }
> +    long count = 0;
> +    char* path;
> +    FILE* f;
> +    asprintf(&path,"tcpip:/dns/%s",argv[1]);

path wird nirgends freigegeben, asprintf macht intern ein malloc

> +    f=fopen(path,"r");
> +    fseek(f,0,SEEK_END);
> +    count = ftell(f);
> +    if(count == 0) {
> +        printf("Error: Abfrage gescheitert!\n");

Nettes Denglisch hier ;-)

> +        return 2;
> +    }
> +    rewind(f);
> +    char* buffer = calloc(count,sizeof(char));

sizeof(char) == 1 per Definition. Gibt es einen Grund, warum kein
malloc? Auf 0 initialisieren ist hier ja nicht sonderlich sinnvoll.

> +    fread(buffer,1,count,f);

Rückgabewert prüfen ist nie eine schlechte Idee.

> +    count /= 16;
> +    fclose(f);
> +    if(count == 0) {
> +        printf("Error: Abfrage gescheitert!\n");
> +        return 3;
> +    }
> +    int i=0;
> +    for(;i < count; i++) {
> +      char* ip = (char*)(buffer+(i*16));
> +      printf("IP Adresse: %s\n",ip);
> +    }
> +    return 0;
> +}
> diff --git a/src/modules/tcpip/dns.c b/src/modules/tcpip/dns.c
> index 0f6e18d..0899fdc 100644
> --- a/src/modules/tcpip/dns.c
> +++ b/src/modules/tcpip/dns.c
> @@ -66,7 +66,7 @@
>  
>  struct dns_cache_entry {
>      char*   name;
> -    dword   ip;
> +    struct dns_request_result* result;
>  };
>  
>  static list_t* dns_cache = NULL;
> @@ -77,7 +77,7 @@ static list_t* dns_cache = NULL;
>   * @return Zum gegebenen Domainnamen passende IP-Adresse oder 0, wenn die
>   * Domain nicht im Cache ist.
>   */
> -static dword dns_cache_get(char* domain)
> +static struct dns_request_result* dns_cache_get(char* domain)
>  {
>      struct dns_cache_entry* entry;
>      int i;
> @@ -87,12 +87,12 @@ static dword dns_cache_get(char* domain)
>      for (i = 0; (entry = list_get_element_at(dns_cache, i)); i++) {
>          if (strcmp(domain, entry->name) == 0) {
>              DEBUG_MSG1(" -- %s: Treffer\n", entry->name);
> -            return entry->ip;
> +            return entry->result;
>          }
>          DEBUG_MSG1(" -- %s: Treffer\n", entry->name);
>      }
> -
> -    return 0;
> +    DEBUG_MSG1(" -- %s: Keine Treffer\n",domain);

Hinter das Komma kommt ein Leerzeichen

> +    return NULL;
>  }
>  
>  /**
> @@ -100,44 +100,63 @@ static dword dns_cache_get(char* domain)
>   * ein alter Eintrag geloescht, um den Cache auf einer ertraeglichen Groesse zu
>   * halten.
>   */
> -static void dns_cache_add(char* domain, dword ip)
> +static void dns_cache_add(char* domain, struct dns_request_result* result)
>  {
>      struct dns_cache_entry* entry = malloc(sizeof(*entry));
>      size_t domain_len = strlen(domain);
>  
> -    entry->ip = ip;
> +    entry->result = result;
>      entry->name = malloc(domain_len + 1);
>      strncpy(entry->name, domain, domain_len);
>      entry->name[domain_len] = '\0';
>  
> -    DEBUG_MSG2("dns: cache_add: %s => %08x\n", entry->name, entry->ip);
> +    DEBUG_MSG1("dns: cache_add: %s\n", entry->name);
>  
>      list_push(dns_cache, entry);
>  
>      if (list_size(dns_cache) > DNS_CACHE_SIZE) {
> +        dns_free_result(list_get_element_at(dns_cache,DNS_CACHE_SIZE));
>          list_remove(dns_cache, DNS_CACHE_SIZE);
>      }
>  }
>  
> +
>  /**
> - * Loest einen Domainnamen in eine IP-Adresse auf.
> + * Gibt die Ressourcen eines dns_request_result wieder frei
> + * @param result Das freizugebene Resultat
> + */
> +void dns_free_result(struct dns_request_result* result) {
> +    // Speicher fuer IP-Adressen freigeben.
> +    free(result->ip_address);
> +    // Speicher fuer das Resultat freigeben.
> +    free(result);
> +}
> +
> +/**
> + * Loest einen Domainnamen in eine IP-Adressen auf.
>   * 
>   * @param domain Der aufzuloesende Domainname als nullterminierter String,
>   * wobei die Labels durch Punkte getrennt sind
>   */
> -dword dns_request(char* domain)
> +struct dns_request_result* dns_request(char* domain)
>  {
> +    // Resultat
> +    struct dns_request_result* dns_result = dns_cache_get(domain);
> +    dword result;

Für neuen Code bevorzugen wir die Typen aus dem C-Standard, also
uint32_t statt dword.

> +
>      // Cache anlegen, falls noch keiner da ist
>      if (dns_cache == NULL) {
> +        DEBUG_MSG("DNS: Erzeuge Cache\n");
>          dns_cache = list_create();
>      }
>  
> -    // Wenn es im Cache ist, daraus nehmen
> -    dword result = dns_cache_get(domain);
> -    if (result) {
> -        return result;
> +    
> +    if(dns_cache_get(domain) != NULL) {
> +      return dns_result;
>      }
>  
> +    dns_result = (struct dns_request_result*)malloc(sizeof(struct dns_request_result));

Überflüssiger Cast.

> +
>      // Ansonsten brauchen wir eine Verbindung zum DNS-Server
>      dword dns_ip = options.nameserver;
>  
> @@ -145,7 +164,7 @@ dword dns_request(char* domain)
>      struct tcp_connection* conn = tcp_open_connection(dns_ip, DNS_PORT);
>      if (conn == NULL) {
>          DEBUG_MSG("DNS: Konnte Verbindung nicht aufbauen.\n");
> -        return 0;
> +        return NULL;
>      }
>  
>      // Puffer fuer die Nachricht reservieren:
> @@ -219,6 +238,8 @@ dword dns_request(char* domain)
>      // Auf Antwort warten
>      DEBUG_MSG("DNS: Warte auf Antwort\n");
>      struct tcp_in_buffer* in_buffer;
> +    // Index fuer dns_result
> +    int dns_result_index = 0;
>      while(1) {
>          if ((in_buffer = list_pop(conn->to_lostio))) {
>              void* reply = in_buffer->data;
> @@ -233,6 +254,11 @@ dword dns_request(char* domain)
>                  big_endian_word(((struct dns_header*) reply)->an_count);
>              reply += sizeof(struct dns_header);
>  
> +            // Speicher fuer die IPs holen
> +            dns_result->ip_address = (dword*)malloc(sizeof(dword) * an_count);

Auch wieder überflüssiger Cast

> +            // Menge speichern
> +            dns_result->ip_count = an_count;
> +
>              // Question Section ignorieren
>              for (i = 0; i < qd_count; i++) {                
>                  // QNAMEs
> @@ -272,13 +298,17 @@ dword dns_request(char* domain)
>  
>                  if (*((word*) reply) == 0x100) {
>                      result = *((dword*) (reply + 10));
> -                    dns_cache_add(domain, result);
> +                    
>                      
>                      // TODO Verbindung schliessen
> -                    return result;
> +                    reply += answer_len;
>                  } else {
> +                    result = 0;
>                      reply += answer_len;
>                  }
> +
> +                dns_result->ip_address[dns_result_index] = result;
> +                dns_result_index++;
>              }
>              
>              break;
> @@ -288,5 +318,7 @@ dword dns_request(char* domain)
>  
>      // TODO Verbindung schliessen
>  
> -    return 0;
> +    dns_cache_add(domain,dns_result);
> +
> +    return dns_result;
>  }
> diff --git a/src/modules/tcpip/include/dns.h b/src/modules/tcpip/include/dns.h
> index cc4281f..d715ce2 100644
> --- a/src/modules/tcpip/include/dns.h
> +++ b/src/modules/tcpip/include/dns.h
> @@ -49,6 +49,11 @@ struct dns_header {
>      uint16_t    ar_count;
>  } __attribute__((packed));
>  
> +struct dns_request_result {
> +    dword* ip_address;
> +    dword ip_count;
> +};
> +
>  
>  // Konstanten f?r die Header-Flags
>  
> @@ -96,6 +101,8 @@ static inline void dns_set_opcode(struct dns_header* header, uint8_t opcode)
>  #define QTYPE_A big_endian_word(1)
>  #define QCLASS_IN big_endian_word(1)
>  
> -dword dns_request(char* domain);
> +struct dns_request_result* dns_request(char* domain);
> +
> +void dns_free_result(struct dns_request_result* result);
>  
>  #endif
> diff --git a/src/modules/tcpip/lostio_if.c b/src/modules/tcpip/lostio_if.c
> index a849f18..7e32b0c 100644
> --- a/src/modules/tcpip/lostio_if.c
> +++ b/src/modules/tcpip/lostio_if.c
> @@ -164,13 +164,30 @@ void lostio_add_device(struct device *device)
>  bool lostio_tcp_not_found(char** path, byte flags, pid_t pid, io_resource_t* ps)
>  {
>      DEBUG_MSG("Knoten anlegen");
> -    
> +   
>      if (strncmp(*path, "/dns/", 5) == 0) {
> -        dword ip = dns_request(strrchr(*path, '/') + 1);
> -        if (ip == 0)
> +        // DNS Request absetzen
> +        struct dns_request_result* result = dns_request(strrchr(*path, '/') + 1);
> +
> +        // Keine IP-Adresse erhalten?
> +        if (result == NULL)
> +            return FALSE;
> +        if (result->ip_count == 0) {
>              return FALSE;
> -        char* data = ip_to_string(ip);
> -        return vfstree_create_node(*path, LOSTIO_TYPES_RAMFILE, strlen(data), data, 0);
> +        }
> +
> +        char* data = calloc(16 * result->ip_count,sizeof(char));
> +        
> +        int i=0;
> +        for(;i < result->ip_count;i++) {
> +            memcpy((char*)(data+(i*16)),ip_to_string(result->ip_address[i]),16);

data ist doch schon char*?

Und soweit ich weiß, macht ip_to_string intern auch wieder ein malloc,
der Rückgabewert müsste also irgendwo freigegeben werden.

> +        }
> +        
> +
> +
> +        // TODO: Was passiert mit data???

Das hängt im Verzeichnisbaum und da hängt es erstmal gut. ;-) Ich
glaube, im Moment löschen wir aus dem DNS-Verzeichnis nie was raus. Wenn
doch, müsste man es an der Stelle wieder freigeben.

> +
> +        return vfstree_create_node(*path, LOSTIO_TYPES_RAMFILE, 16 * result->ip_count * sizeof(char), data, 0);
>      } else {
>          return vfstree_create_node(*path, LOSTIO_TYPES_TCPPORT, 0, NULL, 0);
>      }
> @@ -195,7 +212,13 @@ bool lostio_tcp_pre_open(char** path, byte flags, pid_t pid, io_resource_t* ps)
>      *delim = '\0';
>      dword ip = string_to_ip(*path + 1);
>      if (!ip) {
> -        ip = dns_request(*path + 1);
> +        // DNS Request absetzen
> +        struct dns_request_result* result = dns_request(*path + 1);
> +        if(result != NULL) {
> +            if(result->ip_count > 0) {

if (result && (result->ipcount > 0)) ist übersichtlicher