[RFC] malloced getpw/grxxx functions for bb

tito farmatito at tiscali.it
Sun Sep 14 12:43:11 UTC 2014


Hi,
after a few weeks looking intensively at this code to improve it, shrink it etc,
now i get sick just when I think about it, so i guess it is time to post my results
to the attention of the list members for further improvement.
Attached you will find a drop in replacement for busybox/libpwdgrp/pwd_grp.c
for easy testing as a patch would be big and does not make sense at this stage.

What I've done so far?

/* Rewrite of some parts. Main differences are:
 * 
 * 1) the buffer for getpwuid, getgrgid, getpwnam, getgrnam is dynamically
 *    allocated and reused by later calls. if ERANGE error pops up it is
 *    reallocated to the size of the longest line found in the passwd/group
 *    files and reused for later calls.
 *
 * 2) the passwd/group files:
 *      a) must contain the expected number of fields;
 *      b) some fields are not allowed to be empty (e.g. username, homedir, shell);
 *      c) leading or trailing spaces in fields are not allowed;
 *      d) the string representing uid/gid must be convertible by strtoXX functions;
 *    if the above explained conditions are not met a error message about bad
 *    record in file is printed so that the user knows about it.
 * 3) the internal function for getgrouplist uses a dynamically allocated
 *    buffer and retries with a bigger one in case it is to small;
 * 4) the _r functions use the user supplied buffers that are never reallocated
 *    but use mostly the same common code as the other functions.
 * 5) at the moment only the functions really used by busybox code are
 *    implemented, if you need a particular missing function it should be
 *    easy to write it by using the internal common code.
 */

How was it tested ?

1) at first as standalone out of tree project, by copying the relevant bb stuff
    to it and with simple use cases to see if it somehow works.
2) then with valgrind to catch all the memory errors and leaks that I was
    so good at creating.
3) then in bb's tree where it compiles cleanly and a few test runs of
    relevant apps (adduser, deluser, id etc) seem to work as expected.
4) I've tried also "make test" but it seems to me that there are not
   test for this specific functionalities.

The size?

There is a little size increase (due to the added functionalities and checks?!?)
and I'm not able to shrink it more (I swear I've tried!), maybe others will be able
to do more optimizations. Bloat-o-meter says:

scripts/bloat-o-meter ../busybox_original  ../busybox_new
function                                             old     new   delta
convert_to_struct                                      -     424    +424
parse_common                                           -     214    +214
getgrouplist_internal                                185     312    +127
getpw_common_malloc                                    -      77     +77
getgr_common_malloc                                    -      77     +77
parse_file                                             -      75     +75
getpw_common_r                                         -      73     +73
getgr_common_r                                         -      73     +73
tokenize                                               -      71     +71
.rodata                                           142172  142211     +39
bb_internal_getpwent_r                               102     131     +29
my_pwd                                                 -      28     +28
convert_to_pwd                                         -      25     +25
convert_to_grp                                         -      25     +25
warn_if_error                                          -      18     +18
my_grp                                                 -      16     +16
pwd_buffer_size                                        -       4      +4
pwd_buffer                                             -       4      +4
my_pw                                                  -       4      +4
my_gr                                                  -       4      +4
grp_buffer_size                                        -       4      +4
grp_buffer                                             -       4      +4
gr_off                                                 3       4      +1
sp_off                                                 9       8      -1
ptr_to_statics                                         4       -      -4
bb_internal_getpwuid                                  37      19     -18
bb_internal_getspnam_r                               119      99     -20
bb_internal_getgrgid                                  43      19     -24
bb_internal_getpwnam                                  37      11     -26
get_S                                                 30       -     -30
bb_internal_getgrnam                                  43      11     -32
bb_internal_getpwuid_r                               111       -    -111
bb_internal_getgrgid_r                               111       -    -111
bb__parsepwent                                       112       -    -112
bb_internal_getpwnam_r                               119       -    -119
bb_internal_getgrnam_r                               119       -    -119
bb__parsespent                                       124       -    -124
bb__pgsreader                                        194       -    -194
bb__parsegrent                                       246       -    -246
------------------------------------------------------------------------------
(add/remove: 19/10 grow/shrink: 4/6 up/down: 1416/-1291)      Total: 125 bytes

Is it bug free?

Of course _NOT_ !!!!  but i fixed all the bugs I could find.
Your help is welcome to catch the remaining bugs.

What can you do?

Hint, improvements, critics are as always welcome.

Ciao,
Tito

-------------- next part --------------
A non-text attachment was scrubbed...
Name: pwd_grp.c
Type: text/x-csrc
Size: 16451 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20140914/60021397/attachment.c>


More information about the busybox mailing list