RFC: cleaning up naming mess

Tito farmatito at tiscali.it
Tue Oct 3 09:31:46 UTC 2006


On Monday 2 October 2006 23:52, Denis Vlasenko wrote:
> General guidelines:
> 
> bb_xfunc is sort of stupid. Either bb_ or x, not both.
> 
> xfunc() should closely match libc func() (in params and return value).
> Otherwise use name like func_or_die()
> 
> Mark functions which can die - we will need it when we will
> try to add nofork functionality.
> 
> Use bb_ prefix if otherwise name might look like some libc thing.
> 
> 
> What we currently have and RFC for renaming:
> 
> 
> bb_applet_long_options
> bb_applet_name
> 
> Remove bb_ - names are clearly bbox-related.

Exactly as the names state.
So why remove it?

> 
> bb_default_error_retval

Same as above...
 
> There is nothing "default" about it.
> Rename to xfunc_error_retval.
> 
> 
> bb_error_msg
> bb_error_msg_and_die
> bb_herror_msg
> bb_herror_msg_and_die
> bb_perror_msg
> bb_perror_msg_and_die
> bb_perror_nomsg
> bb_perror_nomsg_and_die
> bb_vherror_msg
> bb_vperror_msg
> bb_info_msg
> bb_vinfo_msg
> bb_verror_msg

Idem.

> Excellent greppability on "error_msg" -> remove bb_, not needed.
> 
> 
> bb_xgetlarg          (should be bb_xgetlarg_bnd)
> bb_xgetlarg10_sfx
> bb_xgetlarg_bnd_sfx
> bb_xgetularg_bnd
> bb_xparse_number     (should be bb_xgetularg10_sfx)
> bb_xgetularg_bnd_sfx
> bb_xgetularg10
> bb_xgetularg10_bnd
> 
> Bad. What do they have to do with _args_? They parse _strings_!

args = commandline args!?

> Also we have tons of unchecked atoi().
> Renaming:
> atoi                                  -> xatoi / xatou    (str)

we have  safe_strtoi(char *arg, int* value) for this.

> bb_xgetlarg (bb_xgetlarg_bnd)         -> xstrtol_range    (str, base, lo, hi)
> bb_xgetlarg10_sfx                     -> xatol_sfx        (str, sfxbtl[])
> bb_xgetlarg_bnd_sfx                   -> xstrtol_range_sfx(str, base, lo, hi, sfxbtl[])
> bb_xgetularg_bnd                      -> xstrtol_range    (str, base, lo, hi)
> bb_xparse_number (bb_xgetularg10_sfx) -> xatoul_sfx       (str, sfxbtl[])
> bb_xgetularg_bnd_sfx                  -> xstrtoul_range   (str, base, lo, hi, sfxbtl[])
> bb_xgetularg10                        -> xatoul           (str)
> bb_xgetularg10_bnd                    -> xatoul_range     (str, lo, hi)

Maybe a little cleanup here is needed, but your proposed names 
are not easier to understand, at least for me (but I'm selfthought....)
> 
> bb_getopt_ulflags

bb_getopt32 ?!

I love the bb_ prefix, makes it easy to know where to look ( in libbb folder...)
rather than trying man this, man that,  google this etc...

> Rename to getopt32. Because abi _has to_ have exact number of bits.
> Just think about applet with 33 different options.
> C code will be _different_ for 32-bit longs and 64-bit longs!
> (If you disagree, show me the code how to handle it).
> 
> bb_opt_complementally

bb_opt32_complementary !?

> Rename to opt_complementary.
> Also maybe introduce global uint32_t option_mask which is set by getopt32.

Yes, good idea.

> Several applets duplicate this already.
> 
> 
> bb_fprintf
> bb_printf
> bb_vfprintf
> bb_vprintf
> 
> Analyze & remove. At first glance these are useless.

Ask Manuel Novoa III  <mjn3 at codepoet.org>.

> 
> bb_fflush_stdout_and_exit
> 
> Remove bb_
> 
> 
> bb_xgetgrnam
> bb_xgetpwnam
> 
> Rename to gid_from_name_or_die, uid_from_name_or_die?
 
bb_getgrgid /* gets a groupname given a gid */
bb_getpwuid /* gets a username given a uid */
bb_getug /* internal function for bb_getpwuid and bb_getgrgid */

> bb_getxxnam

/* indirect dispatcher for pwd helpers.  mostly bb_xgetgrnam and bb_xgetpwnam  */
#include <stdlib.h>
#include "libbb.h"

unsigned long get_ug_id(const char *s,
		long (*__bb_getxxnam)(const char *))
{
	unsigned long r;
	char *p;

	r = strtoul(s, &p, 10);
	if (*p || (s == p)) {
		r = __bb_getxxnam(s);
	}

	return r;
}
#endif /* L_get_ug_id */

> Bad names. What do they do?
> 
> 
> Rest is not reviewed:
> bb_ask_confirmation
> bb_common_bufsiz1
> bb_do_delay
> bb_dump_add
> bb_dump_blocksize
> bb_dump_dump
> bb_dump_fshead
> bb_dump_length
> bb_dump_size
> bb_dump_skip
> bb_dump_vflag
> bb_echo
> bb_fclose_nonstdin
> bb_get_chomped_line_from_file
> bb_get_chunk_from_file
> bb_get_last_path_component
> bb_get_line_from_file
> bb_lookup_host
> bb_lookup_port
> bb_make_directory
> bb_mode_string
> bb_parse_mode
> bb_process_escape_sequence
> bb_setpgrp
> bb_show_usage
> bb_simplify_path
> bb_test
> bb_uuenc_tbl_base64
> bb_uuenc_tbl_std
> bb_uuencode
> bb_warn_ignoring_args
> bb_wfopen
> bb_wfopen_input
> 
> --
> vda

Please if you want to change all our API, wouldn't it be better to change
it gradually so that we can get used to the changes.

BTW.: is it worth the effort, what is the advantage?

Ciao,
Tito



More information about the busybox mailing list