Fabian Schlager <fabian.schlager@gmail.com> wrote:

I wrote a small patch that allows you to specify a file in the config containing forbidden nicks. This may be useful to disallow users changing their nicks to NickServ or ChanServ or any other nick.

Please send patches inline in the future; this makes it easier for me to comment, also please also use diff -p option, it makes it easier to see where the change actually is.

+GLOBAL bool +Conf_ReadBadNicksFile( char *FileName )

const char *

+ // Allocate enough memory to store the nickname + Conf_BadNicks[Conf_BadNicks_Count] = malloc(strlen(tmpNick)); + strcpy(Conf_BadNicks[Conf_BadNicks_Count], tmpNick);

buffer overflow. ( should be malloc(strlen( ) + 1 ). Just use strdup(3).

+ if (Conf_BadNicks_Count > MAX_BADNICKS)

buffer overflow, must be >=

+ if( strcasecmp( Var, "BadNicksFile" ) == 0 ) { + /* File containing restricted nicknames, e.g. NickServ */ + len = strlcpy( Conf_BadNicksFile, Arg, sizeof( Conf_BadNicksFile )); + if (len >= sizeof( Conf_BadNicksFile )) + Config_Error_TooLong( Line, Var );

That looks strange. Why do you try to open it after detecting that the filename was truncated? (Hmm, why do we need to save the filename in the first place?)

+#define MAX_BADNICKS 100 /* Max. count of restricted nicks */

}

- +

This adds whitespace damage. Please don't do this; it only clutters diffsets. (Removing existing trailing whitespace is perfectly ok)

+GLOBAL bool Conf_ReadBadNicksFile PARAMS(( char *FileName ));

I see no reason to export this function; should be static. My take on the intention of this patch: _IF_ there really is demand for such a feature then we should probably implement it without imposing any restrictions, i.e. the nick name list should be as large as needed. On CodingStyle: this is largely open to debate. Basically ngircd has an "old" style (very long functions, long lines, you-know-it-when-you-see-it) and a "new" style (similar to Linux Kernel CodingStyle). Always use the "new" style, even when editing "old-style" code. Comments? Are people interested in the "Nick Blacklist" Option? Florian