Hi Chris, Thank you for your reply to my question. Now, I did checkout freeradius-client from CVS and the function that I asked for exist. However, I did run into a few problems / issues. I am trying to cross-compile this for an ARM Intel ixp425 platform. 1) I needed to export ac_cv_func_uname=no into the environment before executing ./configure. Configure just didn't like the fact that I was cross-compiling with a different toolchain and barfed when it figured it couldn't run the test program. This should probably get fixed for 1.1.6. 2) There are some added macro "DEBUG" added to the freeradius-client.h. This conflicts with a different define of DEBUG defined somewhere else. I'd say either remove it, or rename it. It's for debugging anyway. 3) Cross-compiling 1.1.5, I had a problem with sending authentication requests where the password got corrupted due to some alignment errors in rc_pack_list() and rc_md5_calc(). It appears that the problem is gone in CVS, however; the code looks pretty much the same to me. What I did to circumvent the problem was to declare a char md5[MD5_DIGEST_LENGTH] and use that for MD5Final(), then do a memcpy(output, md5, MD5_DIGEST_LENGTH). This works well, when the output variable doesn't align with the boundary of my processor, in gdb it showed up as 0x...0a and it stomped on 0x...08 (the two first bytes got corrupted and my radiusd didn't understand squat). 4) I looked at the function signature for rc_add_config(...), and it does not seem to require "line", and "filename". It's more an artifact of your static functions requiring some input in case they fail. In my opinion, since the code is shared between rc_read_config(), the static functions should not need to know anything about filename and position, that's a responsibility of the caller function. Return codes can be used to solve this better refactoring the functions to better fit a users (freeradius-client api) need. The filename and line arguments simply doesn't have any value when you embed the radius client. Please consider the following as comments: 5) If you really want to 'embed' freeradius-client, it would be nice if I could specify something similar to "authserv <ip>:<port>:<secret>" and not needing to write a "server" file which declares the servers w/secrets. 6) I don't necessarily understand the radius-protocol in detail, but I would think it would be nice to be able to specify the dictionary attributes/values as you kind of do with rc_add_config(..). This again relates to the fact of reducing external dependencies when embedding freeradius-client. 7) This comment is related to logs, and I haven't studied the use of rc_log too closely. Just in general, this is a library and not an application. It would be nice to possibly use logs to trace calls within the library, but for error reporting it seems like it would be more useful to return an error code, and then call a rc_geterror_str(errno) or something. I am speaking of this as an integrator of OSS and I often like the ability to control the output of an application and not to necessarily "pollute" the system's event logs. BTW, I think freeradius is a great project, keep up the good work :o) I've attached a patch as a set of differences that fixes #2 #4. ---------------------------------------------------------- diff -ruNBb ../../vendor-drop/src/include/freeradius-client.h ../src/include/freeradius-client.h --- ../../vendor-drop/src/include/freeradius-client.h 2007-10-05 08:37:26.000000000 -0700 +++ ../src/include/freeradius-client.h 2007-11-07 08:15:23.000000000 -0800 @@ -17,12 +17,6 @@ #ifndef RADIUSCLIENT_NG_H #define RADIUSCLIENT_NG_H -#ifdef CP_DEBUG -#define DEBUG rc_log -#else -#define DEBUG -#endif - #include <sys/types.h> /* * Include for C99 uintX_t defines is stdint.h on most systems. Solaris uses @@ -447,7 +441,7 @@ SERVER *rc_conf_srv(rc_handle *, char *); int rc_find_server(rc_handle *, char *, uint32_t *, char *); void rc_config_free(rc_handle *); -int rc_add_config(rc_handle *, const char *, const char *, const char *, const int); +int rc_add_config(rc_handle *, const char *, const char *); rc_handle *rc_config_init(rc_handle *); int test_config(rc_handle *, char *); diff -ruNBb ../../vendor-drop/src/lib/config.c ../src/lib/config.c --- ../../vendor-drop/src/lib/config.c 2007-07-11 10:29:29.000000000 -0700 +++ ../src/lib/config.c 2007-11-07 08:16:14.000000000 -0800 @@ -52,7 +52,7 @@ * Returns: 0 on success, -1 on failure */ -static int set_option_str(const char *filename, int line, OPTION *option, const char *p) +static int set_option_str(OPTION *option, const char *p) { if (p) { option->val = (void *) strdup(p); @@ -67,12 +67,11 @@ return 0; } -static int set_option_int(const char *filename, int line, OPTION *option, const char *p) +static int set_option_int(OPTION *option, const char *p) { int *iptr; if (p == NULL) { - rc_log(LOG_ERR, "%s: line %d: bogus option value", filename, line); return -1; } @@ -87,7 +86,7 @@ return 0; } -static int set_option_srv(const char *filename, int line, OPTION *option, const char *p) +static int set_option_srv(OPTION *option, const char *p) { SERVER *serv; char *p_pointer; @@ -100,13 +99,11 @@ p_dupe = strdup(p); if (p_dupe == NULL) { - rc_log(LOG_ERR, "%s: line %d: Invalid option or memory failure", filename, line); return -1; } serv = (SERVER *) option->val; if (serv == NULL) { - DEBUG(LOG_ERR, "option->val / server is NULL, allocating memory"); serv = malloc(sizeof(*serv)); if (serv == NULL) { rc_log(LOG_CRIT, "read_config: out of memory"); @@ -152,12 +149,11 @@ else serv->port[serv->max] = ntohs ((unsigned int) svp->s_port); else { - rc_log(LOG_ERR, "%s: line %d: no default port for %s", filename, line, option->name); if (option->val == NULL) { free(p_dupe); free(serv); } - return -1; + return -2; } } @@ -180,7 +176,7 @@ return 0; } -static int set_option_auo(const char *filename, int line, OPTION *option, const char *p) +static int set_option_auo(OPTION *option, const char *p) { int *iptr; char *p_dupe = NULL; @@ -190,7 +186,6 @@ p_dupe = strdup(p); if (p_dupe == NULL) { - rc_log(LOG_WARNING, "%s: line %d: bogus option value", filename, line); return -1; } @@ -200,18 +195,15 @@ } *iptr = 0; - /*if(strstr(p_dupe,", \t") != NULL) {*/ p_pointer = strtok_r(p_dupe, ", \t", &p_save); - /*}*/ if (!strncmp(p_pointer, "local", 5)) *iptr = AUTH_LOCAL_FST; else if (!strncmp(p_pointer, "radius", 6)) *iptr = AUTH_RADIUS_FST; else { - rc_log(LOG_ERR,"%s: auth_order: unknown keyword: %s", filename, p); free(p_dupe); - return -1; + return -2; } p_pointer = strtok_r(NULL, ", \t", &p_save); @@ -222,9 +214,8 @@ else if ((*iptr & AUTH_LOCAL_FST) && !strcmp(p_pointer, "radius")) *iptr = (*iptr) | AUTH_RADIUS_SND; else { - rc_log(LOG_ERR,"%s: auth_order: unknown or unexpected keyword: %s", filename, p); free(p_dupe); - return -1; + return -2; } } @@ -242,7 +233,7 @@ * Returns: 0 on success, -1 on failure */ -int rc_add_config(rc_handle *rh, const char *option_name, const char *option_val, const char *source, const int line) +int rc_add_config(rc_handle *rh, const char *option_name, const char *option_val) { OPTION *option; @@ -260,22 +251,22 @@ switch (option->type) { case OT_STR: - if (set_option_str(source, line, option, option_val) < 0) { + if (set_option_str(option, option_val) < 0) { return -1; } break; case OT_INT: - if (set_option_int(source, line, option, option_val) < 0) { + if (set_option_int(option, option_val) < 0) { return -1; } break; case OT_SRV: - if (set_option_srv(source, line, option, option_val) < 0) { + if (set_option_srv(option, option_val) < 0) { return -1; } break; case OT_AUO: - if (set_option_auo(source, line, option, option_val) < 0) { + if (set_option_auo(option, option_val) < 0) { return -1; } break; @@ -353,6 +344,7 @@ char buffer[512], *p; OPTION *option; int line; + int rcode; size_t pos; rc_handle *rh; @@ -420,28 +412,46 @@ switch (option->type) { case OT_STR: - if (set_option_str(filename, line, option, p) < 0) { + rcode = set_option_str(option, p); + if (rcode < 0) { + rc_log(LOG_ERR, "%s: line %d: unable to set option value", filename, line ); fclose(configfd); rc_destroy(rh); return NULL; } break; case OT_INT: - if (set_option_int(filename, line, option, p) < 0) { + rcode = set_option_int(option, p); + if (rcode < 0) { + rc_log(LOG_ERR, "%s: line %d: unable to set option value", filename, line); fclose(configfd); rc_destroy(rh); return NULL; } break; case OT_SRV: - if (set_option_srv(filename, line, option, p) < 0) { + rcode = set_option_srv(option, p); + if (rcode == -1) + rc_log(LOG_ERR, "%s: line %d: unable to set option value", filename, line); + + if (rcode == -2) + rc_log(LOG_ERR, "%s: line %d: no default port for %s", filename, line, option->name); + + if (rcode < 0) { fclose(configfd); rc_destroy(rh); return NULL; } break; case OT_AUO: - if (set_option_auo(filename, line, option, p) < 0) { + rcode = set_option_auo(option, p); + if (rcode == -1) + rc_log(LOG_ERR, "%s: line %d: unable to set option value", filename, line); + + if (rcode == -2) + rc_log(LOG_ERR, "%s: line %d: found unknown keyword/value: %s", filename, line, p); + + if (rcode < 0) { fclose(configfd); rc_destroy(rh); return NULL; Binary files ../../vendor-drop/src/lib/.config.c.swp and ../src/lib/.config.c.swp differ diff -ruNBb ../../vendor-drop/src/lib/sendserver.c ../src/lib/sendserver.c --- ../../vendor-drop/src/lib/sendserver.c 2007-08-07 07:26:29.000000000 -0700 +++ ../src/lib/sendserver.c 2007-11-07 07:35:44.000000000 -0800 @@ -226,8 +226,6 @@ /*}*/ } - DEBUG(LOG_ERR, "DEBUG: rc_send_server: creating socket to: %s", server_name); - sockfd = socket (AF_INET, SOCK_DGRAM, 0); if (sockfd < 0) { @@ -297,10 +295,6 @@ auth->length = htons ((unsigned short) total_length); } - DEBUG(LOG_ERR, "DEBUG: local %s : 0, remote %s : %u\n", - inet_ntoa(sinlocal.sin_addr), - inet_ntoa(sinremote.sin_addr), data->svc_port); - for (;;) { sendto (sockfd, (char *) auth, (unsigned int) total_length, (int) 0, Binary files ../../vendor-drop/src/lib/.sendserver.c.swp and ../src/lib/.sendserver.c.swp differ diff -ruNBb ../../vendor-drop/src/src/radembedded.c ../src/src/radembedded.c --- ../../vendor-drop/src/src/radembedded.c 2007-10-08 08:48:06.000000000 -0700 +++ ../src/src/radembedded.c 2007-11-07 08:18:46.000000000 -0800 @@ -52,42 +52,42 @@ /* set auth_order to be radius only */ - if (rc_add_config(rh, "auth_order", "radius", "config", 0) != 0) + if (rc_add_config(rh, "auth_order", "radius") != 0) { printf("ERROR: Unable to set auth_order.\n"); rc_destroy(rh); exit(1); } - if (rc_add_config(rh, "login_tries", "4", "config", 0) != 0) + if (rc_add_config(rh, "login_tries", "4") != 0) { printf("ERROR: Unable to set login_tries.\n"); rc_destroy(rh); exit(1); } - if (rc_add_config(rh, "dictionary", "/usr/local/radius/dictionary", "config", 0) != 0) + if (rc_add_config(rh, "dictionary", "/usr/local/radius/dictionary") != 0) { printf("ERROR: Unable to set dictionary.\n"); rc_destroy(rh); exit(1); } - if (rc_add_config(rh, "seqfile", "/var/run/radius.seq", "config", 0) != 0) + if (rc_add_config(rh, "seqfile", "/var/run/radius.seq") != 0) { printf("ERROR: Unable to set seq file.\n"); rc_destroy(rh); exit(1); } - if (rc_add_config(rh, "radius_retries", "3", "config", 0) != 0) + if (rc_add_config(rh, "radius_retries", "3") != 0) { printf("ERROR: Unable to set radius_retries.\n"); rc_destroy(rh); exit(1); } - if (rc_add_config(rh, "radius_timeout", "5", "config", 0) != 0) + if (rc_add_config(rh, "radius_timeout", "5") != 0) { printf("ERROR: Unable to set radius_timeout.\n"); rc_destroy(rh); @@ -100,14 +100,14 @@ * be used. */ - if (rc_add_config(rh, "authserver", "localhost::testing123", "config", 0) != 0) + if (rc_add_config(rh, "authserver", "localhost::testing123") != 0) { printf("ERROR: Unable to set authserver.\n"); rc_destroy(rh); exit(1); } - if (rc_add_config(rh, "acctserver", "localhost:1813:testing123", "config", 0) != 0) + if (rc_add_config(rh, "acctserver", "localhost:1813:testing123") != 0) { printf("ERROR: Unable to set acctserver.\n"); rc_destroy(rh);