Re: [PATCH] linedit, deluser: use POSIX getpwent instead of getpwent_r
On Fri, Apr 25, 2014 at 9:36 AM, Natanael Copa nc...@alpinelinux.org wrote: Prefer use POSIX getpwent over glibc extension getpwent_r. This fixes building with musl libc with CONFIG_USE_BB_PWD_GRP disabled. Sorry, I missed this patch. How much static data (data or bss increase according to size busybox) do these functions pull in in static build? The comments specifically say they use getpwent_r because that avoids static data size penalty, usually about 1k. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] linedit, deluser: use POSIX getpwent instead of getpwent_r
On Thu, Feb 05, 2015 at 09:42:08PM +0100, Denys Vlasenko wrote:
struct passwd *getpwent()
{
static char *line;
static struct passwd pw;
size_t size=0;
if (!f) f = fopen(/etc/passwd, rbe);
if (!f) return 0;
return __getpwent_a(f, pw, line, size);
}
I would prefer that even struct passwd is malloced...
I don't think it would make much practical difference. It could be
changed though.
But more importantly, bbox can't optimize only for musl.
Other libc'es may have static line buffers there.
And musl will eventually be forced to implement getpwent_r()
if it wants to be usable for more packages... so...
getpwent_r makes no sense; the _r functions are for thread-safe
versions of their corresponding legacy functions, but getpwent_r has
inherent global state -- the iterator. Whoever made it just wasn't
thinking. To make a correct interface like this the caller would need
to have an iterator object to pass to the function, but I can't see
much merit in inventing a new interface for this.
Rich
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: Deluser: deleting the home folder
On Thursday, February 5, 2015, Denys Vlasenko vda.li...@googlemail.com wrote: On Thu, Feb 5, 2015 at 7:48 PM, Laszlo Papp lp...@kde.org wrote: On Thu, Feb 5, 2015 at 6:41 PM, Denys Vlasenko vda.li...@googlemail.com wrote: On Thu, Feb 5, 2015 at 6:49 PM, Laszlo Papp lp...@kde.org wrote: I think it is bad idea to only allow this operation when long options are enabled. Long options and this functionality are two separate things in my book. Standard deluser has only long options. Incompatibility is not a good thing. Sure, but busybox has short option in other cases for such operation, so you need incompatibility somewhere anyway. I think it was a mistake to add incompatible short options to applets whose standard versions had only long options. In addition, the long option is not the problem. The problem is that the functionality is switched off by switching the long option off, rather than the actual functionality. It's about 6.5 kbytes of code in static build. Therefore, busybox does not remain as fine-tunable as possible. There are two extremes: not having any tuning knobs, and having a tuning know for every possible case. Both are bad. The latter one too - you will need to answer to thousands of questions in make config. I tend to add new knobs (CONFIG_xyz) when somebody is pissed enough to complain about it. Do you want to add CONFIG_FEATURE_DELUSER_LONG_OPTIONS? Yes, if you are also happy with that. Your patch otherwise is ok to me, thanks for following this up and apologies if I had been pushy. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: Deluser: deleting the home folder
On Thursday 05 February 2015 19:48:47 Laszlo Papp wrote: On Thu, Feb 5, 2015 at 6:41 PM, Denys Vlasenko vda.li...@googlemail.com wrote: On Thu, Feb 5, 2015 at 6:49 PM, Laszlo Papp lp...@kde.org wrote: I think it is bad idea to only allow this operation when long options are enabled. Long options and this functionality are two separate things in my book. Standard deluser has only long options. Incompatibility is not a good thing. Sure, but busybox has short option in other cases for such operation, so you need incompatibility somewhere anyway. In addition, the long option is not the problem. The problem is that the functionality is switched off by switching the long option off, rather than the actual functionality. Therefore, busybox does not remain as fine-tunable as possible. Which is why I said it was a bad idea in my opinion. I would like to be able to customize busybox to only include things what I want. Currently, if I do not want long options, but I do want this small feature, I have to get everything. This is against the minimal project principles. Hi, i would dare to suggest to use: -r for --remove-home and reserve -R for --remove-all-files (if we ever implement it). this would reduce incompatibility to a mininum. I am aware that this is not Laslo's preferred solution but for me looks better than mixing long options and remove home functionality. Just my 0.2 cents. Ciao, Tito ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] linedit, deluser: use POSIX getpwent instead of getpwent_r
On Thu, Feb 5, 2015 at 9:52 PM, Rich Felker dal...@libc.org wrote: And musl will eventually be forced to implement getpwent_r() if it wants to be usable for more packages... so... getpwent_r makes no sense; The _r part does not make sense; the fact that it avoids static storage _is_ useful. the _r functions are for thread-safe versions of their corresponding legacy functions, but getpwent_r has inherent global state -- the iterator. Whoever made it just wasn't thinking. To make a correct interface like this the caller would need to have an iterator object to pass to the function, but I can't see much merit in inventing a new interface for this. It doesn't matter that it makes no sense. It's glibc compat. If you want more programs rather than less to build against musl, you have to strive to be glibc-compatible where practical. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: Deluser: deleting the home folder
On Wed, Feb 4, 2015 at 4:51 PM, Laszlo Papp lp...@kde.org wrote:
Another desperate ping?
Applied the attached patch.
diff -d -urpN busybox.7/loginutils/deluser.c busybox.8/loginutils/deluser.c
--- busybox.7/loginutils/deluser.c 2014-11-27 23:31:08.0 +0100
+++ busybox.8/loginutils/deluser.c 2015-02-05 18:31:40.066877950 +0100
@@ -11,9 +11,10 @@
*/
//usage:#define deluser_trivial_usage
-//usage: USER
+//usage: IF_LONG_OPTS([--remove-home] ) USER
//usage:#define deluser_full_usage \n\n
//usage: Delete USER from the system
+// --remove-home is self-explanatory enough to put it in --help
//usage:#define delgroup_trivial_usage
//usage: IF_FEATURE_DEL_USER_FROM_GROUP([USER] )GROUP
@@ -37,6 +38,19 @@ int deluser_main(int argc, char **argv)
/* Are we deluser or delgroup? */
int do_deluser = (ENABLE_DELUSER (!ENABLE_DELGROUP || applet_name[3] == 'u'));
+#if !ENABLE_LONG_OPTS
+ const int opt_delhome = 0;
+#else
+ int opt_delhome = 0;
+ if (do_deluser) {
+ applet_long_options =
+ remove-home\0 No_argument \xff;
+ opt_delhome = getopt32(argv, );
+ argv += opt_delhome;
+ argc -= opt_delhome;
+ }
+#endif
+
if (geteuid() != 0)
bb_error_msg_and_die(bb_msg_perm_denied_are_you_root);
@@ -55,10 +69,14 @@ int deluser_main(int argc, char **argv)
case 2:
if (do_deluser) {
/* deluser USER */
- xgetpwnam(name); /* bail out if USER is wrong */
+ struct passwd *pw;
+
+ pw = xgetpwnam(name); /* bail out if USER is wrong */
pfile = bb_path_passwd_file;
if (ENABLE_FEATURE_SHADOWPASSWDS)
sfile = bb_path_shadow_file;
+ if (opt_delhome)
+remove_file(pw-pw_dir, FILEUTILS_RECUR);
} else {
struct group *gr;
do_delgroup:
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: Deluser: deleting the home folder
I think it is bad idea to only allow this operation when long options are enabled. Long options and this functionality are two separate things in my book. On Thu, Feb 5, 2015 at 5:37 PM, Denys Vlasenko vda.li...@googlemail.com wrote: On Wed, Feb 4, 2015 at 4:51 PM, Laszlo Papp lp...@kde.org wrote: Another desperate ping? Applied the attached patch. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: Deluser: deleting the home folder
On Thu, Feb 5, 2015 at 6:49 PM, Laszlo Papp lp...@kde.org wrote: I think it is bad idea to only allow this operation when long options are enabled. Long options and this functionality are two separate things in my book. Standard deluser has only long options. Incompatibility is not a good thing. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] linedit, deluser: use POSIX getpwent instead of getpwent_r
On Thu, Feb 05, 2015 at 02:09:36PM +0100, Denys Vlasenko wrote: On Fri, Apr 25, 2014 at 9:36 AM, Natanael Copa nc...@alpinelinux.org wrote: Prefer use POSIX getpwent over glibc extension getpwent_r. This fixes building with musl libc with CONFIG_USE_BB_PWD_GRP disabled. Sorry, I missed this patch. How much static data (data or bss increase according to size busybox) do these functions pull in in static build? The comments specifically say they use getpwent_r because that avoids static data size penalty, usually about 1k. The static size penalty on musl is a few bytes. We don't have full static buffers for the strings but just use the buffer getline() produces while reading the file, so only this pointer and the passwd struct itself, not the strings it points to, have static storage. Rich ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: Deluser: deleting the home folder
On Thu, Feb 5, 2015 at 6:41 PM, Denys Vlasenko vda.li...@googlemail.com wrote: On Thu, Feb 5, 2015 at 6:49 PM, Laszlo Papp lp...@kde.org wrote: I think it is bad idea to only allow this operation when long options are enabled. Long options and this functionality are two separate things in my book. Standard deluser has only long options. Incompatibility is not a good thing. Sure, but busybox has short option in other cases for such operation, so you need incompatibility somewhere anyway. In addition, the long option is not the problem. The problem is that the functionality is switched off by switching the long option off, rather than the actual functionality. Therefore, busybox does not remain as fine-tunable as possible. Which is why I said it was a bad idea in my opinion. I would like to be able to customize busybox to only include things what I want. Currently, if I do not want long options, but I do want this small feature, I have to get everything. This is against the minimal project principles. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: Deluser: deleting the home folder
On Thu, Feb 5, 2015 at 7:48 PM, Laszlo Papp lp...@kde.org wrote: On Thu, Feb 5, 2015 at 6:41 PM, Denys Vlasenko vda.li...@googlemail.com wrote: On Thu, Feb 5, 2015 at 6:49 PM, Laszlo Papp lp...@kde.org wrote: I think it is bad idea to only allow this operation when long options are enabled. Long options and this functionality are two separate things in my book. Standard deluser has only long options. Incompatibility is not a good thing. Sure, but busybox has short option in other cases for such operation, so you need incompatibility somewhere anyway. I think it was a mistake to add incompatible short options to applets whose standard versions had only long options. In addition, the long option is not the problem. The problem is that the functionality is switched off by switching the long option off, rather than the actual functionality. It's about 6.5 kbytes of code in static build. Therefore, busybox does not remain as fine-tunable as possible. There are two extremes: not having any tuning knobs, and having a tuning know for every possible case. Both are bad. The latter one too - you will need to answer to thousands of questions in make config. I tend to add new knobs (CONFIG_xyz) when somebody is pissed enough to complain about it. Do you want to add CONFIG_FEATURE_DELUSER_LONG_OPTIONS? ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] linedit, deluser: use POSIX getpwent instead of getpwent_r
struct passwd *getpwent()
{
static char *line;
static struct passwd pw;
size_t size=0;
if (!f) f = fopen(/etc/passwd, rbe);
if (!f) return 0;
return __getpwent_a(f, pw, line, size);
}
I would prefer that even struct passwd is malloced...
But more importantly, bbox can't optimize only for musl.
Other libc'es may have static line buffers there.
And musl will eventually be forced to implement getpwent_r()
if it wants to be usable for more packages... so...
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
