Re: A good scripting language for busybox?

[Laszlo Papp]

Why do you need a scripting language in the busybox project?

Cannot you just generate the platform with things like a small subset of
python using Yocto or something similar?

On Fri, Mar 17, 2017 at 3:20 PM, Pavel Aronsky 
wrote:

> Apologies for maybe a wild or off-topic question.
> After dealing with quite a few products with busybox and its ash shell
> used as the primary scripting language, I'd like to ask you, busybox
> experts: what are alternatives?
>
> This page: https://busybox.net/tinyutils.html  - mentions Lua and
> Micro-perl. I'd rather perfer a small subset of Python, but cold not find
> one after a day of googling (this is surprising. I've been sure such things
> exists).
>
> However my search hit one interesting Javascript engine named Duktape (
> duktape.org).
>
> Javascript looks almost as good as Python for me, it is popular and should
> be familiar to new developers. Lua is less familiar, but much better for
> writing moderately simple app logic than the *dreadful* shell language.
>
> So the question: how feasible would be inclusion of Lua or Javascript into
> BB, as option for systems where one of these languages will be heavy used?
>
> As "plan B": has anyone seen (or thought of) a FFI interface for BB that
> would allow to call shared libraries written in C, from ash?
>
> Thanks in advance,
>
> Pavel A.
>
>
>
> ___
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox
>
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Current git HEAD busybox segfaults on some applets

[Laszlo Papp]

Perhaps you could provide the backtrace and strace outputs?

On Fri, Sep 16, 2016 at 12:43 PM, Laurent Bercot 
wrote:

> On 15/09/2016 13:55, Denys Vlasenko wrote:
>
>> Works for me...
>>
>
>  I can reproducibly segfault when wgetting a page twice (i.e. the file
> already exists).
>  I can also reproducibly segfault when using busybox vi, for instance
> on the index.html file downloaded with "wget http://skarnet.org/;
>
>  I have tried building with 2 different toolchains that have always
> worked for me, including with previous versions of busybox, and both
> binaries segfault.
>
>  Linux-4.6.3, musl 1.1.15, x86_64.
>
>  I will try on another machine.
>
> --
>  Laurent
>
>
> ___
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox
>
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: two small bugs in the shell

[Laszlo Papp]

On Thu, Jan 21, 2016 at 10:27 AM, Ron Yorston  wrote:

> >and the same applies to echo
>
> Busybox echo follows GNU conventions for escape handling rather than
> POSIX.


Is that a good thing ?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: adduser: add -k [was: Re: [OE-core] Post USERADD_PARAM action]

[Laszlo Papp]

On Mon, May 25, 2015 at 10:32 PM, Bernhard Reutner-Fischer
rep.dot@gmail.com wrote:
 On Fri, May 22, 2015 at 12:37:13PM +0100, Laszlo Papp wrote:
 Hi,

 is it possible to do some post action after useradd_param is executed?

 I would like to modify the created user's home directory, in this
 special case the profile. I see that the useradd util has this on
 desktop, but busybox does not have it, and very likely will not have
 it, at least not in the near future anyway:

 mhm, why so?

People tend to complain about not very common and minimalistic
features going in. Perhaps, if it is optional, it may sneak in easier.
Man proposes, Denys disposes.


 The attached compiles for me.

 cheers,


-k, --skel SKEL_DIR
The skeleton directory, which contains files and
 directories to be copied in the user's home directory, when the home
 directory is created by useradd.

This option is only valid if the -m (or --create-home)
 option is specified.

If this option is not set, the skeleton directory is
 defined by the SKEL variable in /etc/default/useradd or, by default,
 /etc/skel.

If possible, the ACLs and extended attributes are copied.

 I would like to do something like this with the created user's home 
 directory:

 echo PATH=/sbin:$PATH  /home/foo/.profile
 chmod foo:foo /home/foo/.profile

 But then again, for this, I need to make sure that the user is already 
 created.

 Ys, L.
 --
 ___
 Openembedded-core mailing list
 openembedded-c...@lists.openembedded.org
 http://lists.openembedded.org/mailman/listinfo/openembedded-core
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: mailing list messages

[Laszlo Papp]

Well, this happened to me with another mailing list. I would suggest
to change your credentials to start with. It looks suspicious.

On Wed, Apr 22, 2015 at 1:40 PM, tito farmat...@tiscali.it wrote:
 On Wednesday 22 April 2015 14:38:44 you wrote:

 Was your email over quota or perhaps the mail server was down?





 Maybe the server was down'cause I've not sent mails in the last days.



 On April 22, 2015 8:27:15 AM EST, tito farmat...@tiscali.it wrote:

 Hi,

 today I've got this message:

 

 Your membership in the mailing list busybox has been disabled due to

 excessive bounces The last bounce received from you was dated

 22-Apr-2015. You will not get any more messages from this list until

 you re-enable your membership. You will receive 2 more reminders like

 this before your membership in the list is deleted.

 

 and I wonder what bounces it is talking about?

 

 Ciao,

 Tito

 

 






 ___
 busybox mailing list
 busybox@busybox.net
 http://lists.busybox.net/mailman/listinfo/busybox
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Tar/gzip Commands: Busybox vs. GNU (error: Invalid magic)

[Laszlo Papp]

Dear Simon Chen,

This is very poor problem explanation, I am afraid. Please read this:
http://www.catb.org/esr/faqs/smart-questions.html

Best Regards, L.

On Thu, Mar 19, 2015 at 8:34 PM, Chen, Simon (N-DCR SYSTEM HOUSE)
simon.c...@lmco.com wrote:
 Hi,



 I am working on an embedded OS called YOCTO, which apparently includes
 BusyBox into its build. YOCTO is currently running on a target board, and I
 am trying to integrate an AMD Radeon E8860 GPU into it. This requires
 installation of a driver called “fglrx”, otherwise also known as AMD
 Catalyst. The driver installer (which is a .run) uses BusyBox “gzip –cd” and
 “tar” commands to extract an undisclosed internal file, which returned the
 error message “Invalid magic.” This does not happen when I try running the
 driver installer on a desktop UBUNTU OS, which uses GNU. I was wondering
 what the key differences between GNU’s and Busybox’s extraction functions
 were, and what I could do to fix this error.



 Thanks,

 Simon Chen


 ___
 busybox mailing list
 busybox@busybox.net
 http://lists.busybox.net/mailman/listinfo/busybox
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: RFD: Rework/extending functionality of mdev

[Laszlo Papp]

On Wed, Mar 11, 2015 at 7:02 PM, Harald Becker ra...@gmx.de wrote:
 On 11.03.2015 16:21, Laurent Bercot wrote:

 I don't understand that binary choice... You can work on your own
 project without forking Busybox. You can use both Busybox and your
 own project on your systems. Busybox is a set of tools, why should it
 be THE set of tools ?


 Sure, I know how to do this, I started creating adapted Busybox versions to
 specific needs for a minimalistic 386SX Board. Around 1995, or so ... wow,
 long time now :)

 It is neither a knowledge nor any technical problem, it is preference:
 I want to have *one* statical binary in the minimal system, and being
 able to run a full system setup with this (system) binary (or call it
 tool set). All I need then is the binary, some configs, some scripts
 (and may be special applications). I even go so far, to run a system
 with exactly that one binary only, all other applications functions are
 done with scripting (ash, sed, awk). Sure those are minimalist
 (dedicated systems), but they may be used in a comfortable manner.

 I even started a project to create a file system browser (comparable to
 Midnight Commander, with no two pane mode but up to 9 quick switch
 directories), using only BB and scripting. All packed in a (possibly self
 extracting) single shell script. The only requirements to run this, is
 (should be) a working BB (defconfig) environment, usual proc, sys, dev,
 setup and a writable /tmp directory (e.g. on tmpfs). The work for this was
 half way through to first public alpha, then Denys reaction on a slight
 change request was so frustrating that I pushed the project into an
 otherwise unused archive corner, and stopped any further development.


 I'm not sure how heavily mdev [-s] relies on libbb and how hard it
 would be to extract the source and make it into a standalone, easily
 hackable project, but if you want to test architecture ideas, that's
 the way to go - copy stuff and tinker with it until you have
 something satisfying.


 I always did it this way, and never posted untested stuff, except some
 snippets when one asked for something and I quickly hacked something for
 an answer (with mark set as untested).


 ... if not, you still have your harald-mdev, and you can still use it
 along with Busybox - you'll have two binaries instead of one, but
 even on whatever tiny noMMU you can work on, you'll be fine.


 Sure, I could have two, three, four, ten, twenty, hundred, ... programs, but
 my preference is to have *one* statically linked binary for the complete
 system tool set (on minimal systems).

So why don't you write such a binary wrapping busybox then and other
things? I think KISS principle still ought to be alive these days. Not
to mention, Denys already cannot cope with the maintenance the way
that it would be ideal. For instance, some of my IMHO serious bugfixes
are uncommented. Putting more and more stuff into busybox would just
make the situation worse and more frustrating, sorry. I really do not
want busybox to follow the systemd way. On the positive side of
systemd, they at least have far more resource than what Denys can
offer, at least in my opinion, so ...

 ... Thats the reason why I dislike and don't use your system approach :( ...
 otherwise great work :)


 That does not preclude design discussions, which I like, and which
 can happen here (unless moderators object), and people like Isaac,
 me, or obviously Denys, wouldn't be so defensive - because it's now
 about your project, not Busybox; you have the final say, and it's
 totally fine, because I don't have to use harald-mdev if I don't want
 to.


 One of the things I really hate, is to force someone doing something
 (especially in a specific way), only topped by someone else forcing me to do
 something in a specific way :( ...

 ... so I always try to do modifications in a way which let others decide
 about usage, expecting not to break existing setups (at least without asking
 ahead if welcome). Slight modifications may happen from modifications (e.g.
 different parameter notion), if unavoidable, but they shall not require
 complete changes in system setup.

 --
 Harald

 ___
 busybox mailing list
 busybox@busybox.net
 http://lists.busybox.net/mailman/listinfo/busybox
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: RFD: Rework/extending functionality of mdev

[Laszlo Papp]

On Thu, Mar 12, 2015 at 6:00 PM, Harald Becker ra...@gmx.de wrote:
 Hi Laszlo !

 So why don't you write such a binary wrapping busybox then and other
 things? I think KISS principle still ought to be alive these days.
 Not to mention, Denys already cannot cope with the maintenance the
 way that it would be ideal. For instance, some of my IMHO serious
 bugfixes are uncommented. Putting more and more stuff into busybox
 would just make the situation worse and more frustrating,


 The usual way of development is:

 (1) Planning the work to do (the step we are discussing here)

 (2) Code hacking (what I will do next)

 (3) Preliminary Testing (also mine job)

 (4) Offer access to those who like (for further testing)

 (5) fixing complains

 (6) putting into main stream (or accessible by the rest)

 Right? So what is your complain?


 sorry. I really do not want busybox to follow the systemd way.


 Who told you, I'm trying to go that way. My intention is to overcome the
 mdev problems, and allow those who like to use the netlink interface. I
 dislike encoding any fixed functionality in a binary, and don't force
 anybody to use possible extensions.

 Laurent poked me to more clarity, which would mean to split early
 initialization from mdev operation, with the cave eat of a slight change in
 init scripts (may be one more command or some extra
 command parameter, could be done automatic, but than different
 functionalities in applet - may be more discussion required on that). Beside
 that, it's shall be up to the system maintainer, to chose which device
 management mechanism to use (BB shall provide the tools for that, small
 modular tools, bound together by admin - no big monolithic).


 On the positive side of systemd, they at least have far more resource
 than what Denys can offer, at least in my opinion, so ...


 You are talking about development resources? Here they are! I'm willing to
 do that job, not asking for someone else doing the work.

It is not only about feature development resources, but also
maintenance. Denys will be the maintainer for new busybox code as far
as I am aware. I have not seen this model changing for a very long
while, sadly.

Once, I tried to ask for changing that model, but I was apparently
just shooting myself in the foot based on the feedback.

It is nice that you are trying to help and I certainly appreciate it,
but why cannot you simply do that job nicely outside busybox where
*you* have to be responsible for that project? It would be an explicit
way of enforcing KISS and not putting more burden on Denys.

He may enjoy maintaining more and more code, but in all honesty with
due respect and appreciation, I do not enjoy when developers do not
get response for their patches and they kind of all depend only on
Denys with regards to upstreaming.

It is also a bit demotivating that we do not get early feedback and
then we realize that our patches are completely reworked by Denys. It
is unfortunate that our work almost goes to /dev/null. I do not feel
appreciated.

This is all happening because of more and more complex stuff to be
maintained by one person. It is not a personal offense against Denys
to be fair. It is a model I very much disagree with in general.

If you can convince the busybox community to split up the
maintainership, perhaps that would be a completely different
discussion to start with, but in all honesty, I do not like these
monolythic projects. I still stick by that KISS is a good thing. If
I could, I would personally replace busybox with little custom tools
on my system, but I currently do not have the resource for that.
Therefore, all the complexities and non-kiss that goes in is something
I need to accept.


 I'm asking about, which preferences other people have, so I'm able to get
 the right decisions, before I start hacking code ... so what's wrong?

Asking for feedback is good, nothing wrong in there; putting this into
busybox this way is wrong on the other hand IMHO.


 --
 Harald

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: ntpd daemon

[Laszlo Papp]

On Thu, Mar 5, 2015 at 12:19 AM, Isaac Dunham ibid...@gmail.com wrote:
 On Wed, Mar 04, 2015 at 10:05:06PM +, Laszlo Papp wrote:
 Better, thanks, but the documentation would still not mention the case of
 abort.

 On Tuesday, March 3, 2015, Isaac Dunham ibid...@gmail.com wrote:
  How is this patch for clarifying the documentation?

  //usage:   )
 +//usage: \n   If no peer is defined, ntpd will not run
  //usage:   IF_FEATURE_NTPD_SERVER(
 +//usage: \n   unless ntpd is running as a server (-l)
 snip

 Are you sure?
 I didn't use the word abort, but it seemed to me that if no peer
 is defined, ntpd will not run should make the behavior clear.

The problem is not when no peer is defined. The problem is when the
peer is defined, but we cannot talk to it. Therefore, the issue that I
raised is still addressed with regards to this.


 Thanks,
 Isaac Dunham
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: ntpd daemon

[Laszlo Papp]

On Thu, Mar 5, 2015 at 10:43 AM, Laszlo Papp lp...@kde.org wrote:
 On Thu, Mar 5, 2015 at 12:19 AM, Isaac Dunham ibid...@gmail.com wrote:
 On Wed, Mar 04, 2015 at 10:05:06PM +, Laszlo Papp wrote:
 Better, thanks, but the documentation would still not mention the case of
 abort.

 On Tuesday, March 3, 2015, Isaac Dunham ibid...@gmail.com wrote:
  How is this patch for clarifying the documentation?

  //usage:   )
 +//usage: \n   If no peer is defined, ntpd will not run
  //usage:   IF_FEATURE_NTPD_SERVER(
 +//usage: \n   unless ntpd is running as a server (-l)
 snip

 Are you sure?
 I didn't use the word abort, but it seemed to me that if no peer
 is defined, ntpd will not run should make the behavior clear.

 The problem is not when no peer is defined. The problem is when the
 peer is defined, but we cannot talk to it. Therefore, the issue that I
 raised is still addressed with regards to this.

... still _not_ addressed ...

I mean, the alternative behavior would be to keep trying, but the
author of the code decided to abort. This is not an insignificant
detail. The end user would need to be aware of the proper operation.



 Thanks,
 Isaac Dunham
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: ntpd daemon

[Laszlo Papp]

Better, thanks, but the documentation would still not mention the case of
abort.

On Tuesday, March 3, 2015, Isaac Dunham ibid...@gmail.com wrote:
 On Tue, Mar 03, 2015 at 03:58:12PM +, Laszlo Papp wrote:
 On Tue, Mar 3, 2015 at 3:54 PM, Denys Vlasenko vda.li...@googlemail.com
wrote:
  On Tue, Mar 3, 2015 at 2:56 PM, Laszlo Papp lp...@kde.org wrote:
  On Mon, Mar 2, 2015 at 5:07 PM, Denys Vlasenko 
vda.li...@googlemail.com wrote:
  On Mon, Mar 2, 2015 at 10:39 AM, Laszlo Papp lp...@kde.org wrote:
  Either way, since I have spent a couple of hours before realizing
it,
  I think it would be beneficial to briefly extend documentation. It
may
  sound trivial for people familiar enough with ntp, but for me it
was
  not. Usually, in networking, servers and clients are different
things,
  not rarely significantly. You can call me silly or any other way
that
  you like, but it would have helped me to have a few more words about
  the usage of this option.
 
  Propose a change to ntpd --help
 
  Apparently you did that yourself:
 
 
http://git.busybox.net/busybox/commit/?id=3aef814c0b08d9703280b4772060ce5016c683c4
 
  I think there are still at least two issues in the peer documentation:
 
  -p PEER Obtain time from PEER (may be repeated)
  If -p is not given, read /etc/ntp.conf
 
  1) It does not mention that it aborts if the peer is unavailable, e.g.
  unresolvable hostname.
  2) It does not mention what the format of /etc/ntp.conf is. It may be
  obvious for you, but for me it is not. I assume it is some standard
  format, but what is the standard? Any reference to it, etc?
 
  It's standard ntpd compat. google man ntp.conf

 I know, but that is not the point I am trying to make. Reading the
 help output without investigating about the code, it is unclear for
 the user what format the file has. It can be standard, it can be
 busybox specific, etc.

 Also, do not forget about the other point. That is also important and
 well-hidden.

 How is this patch for clarifying the documentation?



___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: ntpd daemon

[Laszlo Papp]

On Tue, Mar 3, 2015 at 3:54 PM, Denys Vlasenko vda.li...@googlemail.com wrote:
 On Tue, Mar 3, 2015 at 2:56 PM, Laszlo Papp lp...@kde.org wrote:
 On Mon, Mar 2, 2015 at 5:07 PM, Denys Vlasenko vda.li...@googlemail.com 
 wrote:
 On Mon, Mar 2, 2015 at 10:39 AM, Laszlo Papp lp...@kde.org wrote:
 Either way, since I have spent a couple of hours before realizing it,
 I think it would be beneficial to briefly extend documentation. It may
 sound trivial for people familiar enough with ntp, but for me it was
 not. Usually, in networking, servers and clients are different things,
 not rarely significantly. You can call me silly or any other way that
 you like, but it would have helped me to have a few more words about
 the usage of this option.

 Propose a change to ntpd --help

 Apparently you did that yourself:

 http://git.busybox.net/busybox/commit/?id=3aef814c0b08d9703280b4772060ce5016c683c4

 I think there are still at least two issues in the peer documentation:

 -p PEER Obtain time from PEER (may be repeated)
 If -p is not given, read /etc/ntp.conf

 1) It does not mention that it aborts if the peer is unavailable, e.g.
 unresolvable hostname.
 2) It does not mention what the format of /etc/ntp.conf is. It may be
 obvious for you, but for me it is not. I assume it is some standard
 format, but what is the standard? Any reference to it, etc?

 It's standard ntpd compat. google man ntp.conf

I know, but that is not the point I am trying to make. Reading the
help output without investigating about the code, it is unclear for
the user what format the file has. It can be standard, it can be
busybox specific, etc.

Also, do not forget about the other point. That is also important and
well-hidden.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Bug in the deluser applet

[Laszlo Papp]

Denys, any feedback about this bugfix?

On Thu, Feb 19, 2015 at 6:16 PM, Laszlo Papp lp...@kde.org wrote:
 From b03ad793d1188148953fa280dda672229b9a6524 Mon Sep 17 00:00:00 2001
 From: Laszlo Papp laszlo.p...@polatis.com
 Date: Wed, 18 Feb 2015 15:20:58 +
 Subject: [PATCH] Delete the user from all the groups for user deletion

 ---
  libbb/update_passwd.c | 57 
 +++
  loginutils/deluser.c  |  4 +++-
  2 files changed, 42 insertions(+), 19 deletions(-)

 diff --git a/libbb/update_passwd.c b/libbb/update_passwd.c
 index a30af6f..df544f0 100644
 --- a/libbb/update_passwd.c
 +++ b/libbb/update_passwd.c
 @@ -62,6 +62,8 @@ static void check_selinux_update_passwd(const char 
 *username)
  only if CONFIG_PASSWD=y and applet_name[0] == 'p' like in passwd
  or if CONFIG_CHPASSWD=y and applet_name[0] == 'c' like in chpasswd

 + 8) delete a user from all groups : update_passwd(FILE, NULL, NULL, MEMBER)
 +
   This function does not validate the arguments fed to it
   so the calling program should take care of that.

 @@ -81,7 +83,7 @@ int FAST_FUNC update_passwd(const char *filename,
 FILE *new_fp;
 char *fnamesfx;
 char *sfx_char;
 -   char *name_colon;
 +   char *name_colon = 0;
 unsigned user_len;
 int old_fd;
 int new_fd;
 @@ -99,13 +101,15 @@ int FAST_FUNC update_passwd(const char *filename,
 if (filename == NULL)
 return ret;

 -   check_selinux_update_passwd(name);
 +   if (name) check_selinux_update_passwd(name);

 /* New passwd file, /etc/passwd+ for now */
 fnamesfx = xasprintf(%s+, filename);
 sfx_char = fnamesfx[strlen(fnamesfx)-1];
 -   name_colon = xasprintf(%s:, name);
 -   user_len = strlen(name_colon);
 +if (name) {
 +name_colon = xasprintf(%s:, name);
 +user_len = strlen(name_colon);
 +}

 if (shadow)
 old_fp = fopen(filename, r+);
 @@ -162,21 +166,38 @@ int FAST_FUNC update_passwd(const char *filename,
 /* Read current password file, write updated /etc/passwd+ */
 changed_lines = 0;
 while (1) {
 -   char *cp, *line;
 -
 -   line = xmalloc_fgetline(old_fp);
 -   if (!line) /* EOF/error */
 -   break;
 -   if (strncmp(name_colon, line, user_len) != 0) {
 -   fprintf(new_fp, %s\n, line);
 -   goto next;
 -   }
 -
 -   /* We have a match with name:... */
 -   cp = line + user_len; /* move past name: */
 +char *cp, *line;
 +if (!name  member) {
 +struct group* g;
 +if ((g = getgrent())) {
 +char gline[LINE_MAX];
 +char **s= g-gr_mem;
 +bool sep = false;
 +snprintf(gline, sizeof(gline), %s:%s:%i:,
 g-gr_name, g-gr_passwd, g-gr_gid);
 +while (*s) {
 +if (strcmp(*s, member)) { if (sep) strcat(gline,
 ,); else sep = true; strcat(gline, *s); }
 +++s;
 +}
 +fprintf(new_fp, %s\n, gline);
 +continue;
 +} else {
 +break;
 +}
 +} else {
 +line = xmalloc_fgetline(old_fp);
 +if (!line) /* EOF/error */
 +break;
 +if (!name_colon || strncmp(name_colon, line, user_len) != 0) {
 +fprintf(new_fp, %s\n, line);
 +goto next;
 +}
 +
 +/* We have a match with name:... */
 +cp = line + user_len; /* move past name: */
 +}

  #if ENABLE_FEATURE_ADDUSER_TO_GROUP || ENABLE_FEATURE_DEL_USER_FROM_GROUP
 -   if (member) {
 +   if (name  member) {
 /* It's actually /etc/group+, not /etc/passwd+ */
 if (ENABLE_FEATURE_ADDUSER_TO_GROUP
   applet_name[0] == 'a'
 @@ -240,7 +261,7 @@ int FAST_FUNC update_passwd(const char *filename,

 if (changed_lines == 0) {
  #if ENABLE_FEATURE_ADDUSER_TO_GROUP || ENABLE_FEATURE_DEL_USER_FROM_GROUP
 -   if (member) {
 +   if (name  member) {
 if (ENABLE_ADDGROUP  applet_name[0] == 'a')
 bb_error_msg(can't find %s in %s,
 name, filename);
 if (ENABLE_DELGROUP  applet_name[0] == 'd')
 diff --git a/loginutils/deluser.c b/loginutils/deluser.c
 index 01a9386..a3f5f3a 100644
 --- a/loginutils/deluser.c
 +++ b/loginutils/deluser.c
 @@ -82,6 +82,9 @@ int deluser_main(int argc, char **argv)
   do_delgroup:
 /* delgroup GROUP or delgroup USER GROUP */
 if (do_deluser  0) { /* delgroup after deluser? */
 +pfile = bb_path_group_file;
 +if (update_passwd(pfile, NULL

Re: ntpd daemon

[Laszlo Papp]

On Mon, Mar 2, 2015 at 5:07 PM, Denys Vlasenko vda.li...@googlemail.com wrote:
 On Mon, Mar 2, 2015 at 10:39 AM, Laszlo Papp lp...@kde.org wrote:
 Either way, since I have spent a couple of hours before realizing it,
 I think it would be beneficial to briefly extend documentation. It may
 sound trivial for people familiar enough with ntp, but for me it was
 not. Usually, in networking, servers and clients are different things,
 not rarely significantly. You can call me silly or any other way that
 you like, but it would have helped me to have a few more words about
 the usage of this option.

 Propose a change to ntpd --help

Apparently you did that yourself:

http://git.busybox.net/busybox/commit/?id=3aef814c0b08d9703280b4772060ce5016c683c4

I think there are still at least two issues in the peer documentation:

-p PEER Obtain time from PEER (may be repeated)
If -p is not given, read /etc/ntp.conf

1) It does not mention that it aborts if the peer is unavailable, e.g.
unresolvable hostname.
2) It does not mention what the format of /etc/ntp.conf is. It may be
obvious for you, but for me it is not. I assume it is some standard
format, but what is the standard? Any reference to it, etc?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: ntpd daemon

[Laszlo Papp]

On Sun, Mar 1, 2015 at 5:25 PM, Denys Vlasenko vda.li...@googlemail.com wrote:
 On Sun, Mar 1, 2015 at 2:04 PM, Laszlo Papp lp...@kde.org wrote:
 Hi,

 when I run the daemon with the following option, does it still accept
 the PEER parameter properly to function as a client as well as server?

 -l  Run as server on port 123

 Yes, it does.

Yes, you are right. Sorry about not giving any update yesterday. I
checked this source code yesterday which made it clear for me:

http://git.busybox.net/busybox/tree/networking/ntpd.c#n2180

and

http://git.busybox.net/busybox/tree/networking/ntpd.c#n2355

The globals are pretty ugly, though, but choices ... choices ...

Either way, since I have spent a couple of hours before realizing it,
I think it would be beneficial to briefly extend documentation. It may
sound trivial for people familiar enough with ntp, but for me it was
not. Usually, in networking, servers and clients are different things,
not rarely significantly. You can call me silly or any other way that
you like, but it would have helped me to have a few more words about
the usage of this option.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

ntpd daemon

[Laszlo Papp]

Hi,

when I run the daemon with the following option, does it still accept
the PEER parameter properly to function as a client as well as server?

-l  Run as server on port 123

The code is bit too complex, so I could not yet get my head around to it.

The reason why I am asking this because I am unsure whether I ought to
run two instances if I want to get my board to function as a server as
well a client to a remote ntpd server.

I have wasted some time on it, so perhaps if the case is that it could
be a server and a client simultaneously with the -l option, a short
extension could be made to the usage documentation?

Thank you in advance.

Cheers, L.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Bug in the deluser applet

[Laszlo Papp]

From b03ad793d1188148953fa280dda672229b9a6524 Mon Sep 17 00:00:00 2001
From: Laszlo Papp laszlo.p...@polatis.com
Date: Wed, 18 Feb 2015 15:20:58 +
Subject: [PATCH] Delete the user from all the groups for user deletion

---
 libbb/update_passwd.c | 57 +++
 loginutils/deluser.c  |  4 +++-
 2 files changed, 42 insertions(+), 19 deletions(-)

diff --git a/libbb/update_passwd.c b/libbb/update_passwd.c
index a30af6f..df544f0 100644
--- a/libbb/update_passwd.c
+++ b/libbb/update_passwd.c
@@ -62,6 +62,8 @@ static void check_selinux_update_passwd(const char *username)
 only if CONFIG_PASSWD=y and applet_name[0] == 'p' like in passwd
 or if CONFIG_CHPASSWD=y and applet_name[0] == 'c' like in chpasswd

+ 8) delete a user from all groups : update_passwd(FILE, NULL, NULL, MEMBER)
+
  This function does not validate the arguments fed to it
  so the calling program should take care of that.

@@ -81,7 +83,7 @@ int FAST_FUNC update_passwd(const char *filename,
FILE *new_fp;
char *fnamesfx;
char *sfx_char;
-   char *name_colon;
+   char *name_colon = 0;
unsigned user_len;
int old_fd;
int new_fd;
@@ -99,13 +101,15 @@ int FAST_FUNC update_passwd(const char *filename,
if (filename == NULL)
return ret;

-   check_selinux_update_passwd(name);
+   if (name) check_selinux_update_passwd(name);

/* New passwd file, /etc/passwd+ for now */
fnamesfx = xasprintf(%s+, filename);
sfx_char = fnamesfx[strlen(fnamesfx)-1];
-   name_colon = xasprintf(%s:, name);
-   user_len = strlen(name_colon);
+if (name) {
+name_colon = xasprintf(%s:, name);
+user_len = strlen(name_colon);
+}

if (shadow)
old_fp = fopen(filename, r+);
@@ -162,21 +166,38 @@ int FAST_FUNC update_passwd(const char *filename,
/* Read current password file, write updated /etc/passwd+ */
changed_lines = 0;
while (1) {
-   char *cp, *line;
-
-   line = xmalloc_fgetline(old_fp);
-   if (!line) /* EOF/error */
-   break;
-   if (strncmp(name_colon, line, user_len) != 0) {
-   fprintf(new_fp, %s\n, line);
-   goto next;
-   }
-
-   /* We have a match with name:... */
-   cp = line + user_len; /* move past name: */
+char *cp, *line;
+if (!name  member) {
+struct group* g;
+if ((g = getgrent())) {
+char gline[LINE_MAX];
+char **s= g-gr_mem;
+bool sep = false;
+snprintf(gline, sizeof(gline), %s:%s:%i:,
g-gr_name, g-gr_passwd, g-gr_gid);
+while (*s) {
+if (strcmp(*s, member)) { if (sep) strcat(gline,
,); else sep = true; strcat(gline, *s); }
+++s;
+}
+fprintf(new_fp, %s\n, gline);
+continue;
+} else {
+break;
+}
+} else {
+line = xmalloc_fgetline(old_fp);
+if (!line) /* EOF/error */
+break;
+if (!name_colon || strncmp(name_colon, line, user_len) != 0) {
+fprintf(new_fp, %s\n, line);
+goto next;
+}
+
+/* We have a match with name:... */
+cp = line + user_len; /* move past name: */
+}

 #if ENABLE_FEATURE_ADDUSER_TO_GROUP || ENABLE_FEATURE_DEL_USER_FROM_GROUP
-   if (member) {
+   if (name  member) {
/* It's actually /etc/group+, not /etc/passwd+ */
if (ENABLE_FEATURE_ADDUSER_TO_GROUP
  applet_name[0] == 'a'
@@ -240,7 +261,7 @@ int FAST_FUNC update_passwd(const char *filename,

if (changed_lines == 0) {
 #if ENABLE_FEATURE_ADDUSER_TO_GROUP || ENABLE_FEATURE_DEL_USER_FROM_GROUP
-   if (member) {
+   if (name  member) {
if (ENABLE_ADDGROUP  applet_name[0] == 'a')
bb_error_msg(can't find %s in %s,
name, filename);
if (ENABLE_DELGROUP  applet_name[0] == 'd')
diff --git a/loginutils/deluser.c b/loginutils/deluser.c
index 01a9386..a3f5f3a 100644
--- a/loginutils/deluser.c
+++ b/loginutils/deluser.c
@@ -82,6 +82,9 @@ int deluser_main(int argc, char **argv)
  do_delgroup:
/* delgroup GROUP or delgroup USER GROUP */
if (do_deluser  0) { /* delgroup after deluser? */
+pfile = bb_path_group_file;
+if (update_passwd(pfile, NULL, NULL, name) == -1)
+return EXIT_FAILURE;
gr = getgrnam(name);
if (!gr)
return EXIT_SUCCESS;
@@ -99,7

Re: Bug in the deluser applet

[Laszlo Papp]

On Wed, Feb 18, 2015 at 3:37 PM, Laszlo Papp lp...@kde.org wrote:
 Actually, the locking and swapping logic would be common anyway which
 already resides in update_passwd, so if that reusable component is not
 moved to a separate function to be reused, we could use put our
 conditional magic into the while loop. I am preparing with that change
 now.

 Please check the patch below. I tested it and it works for me. The
 coding style may not suit the existing busybox coding style, but I do
 not mind that for now as I will use the patch as is for now in my
 local fork anyhow. Any (non-stylistic) feedback is welcome.

 commit 22a7da560af82c33ca9334039522e9a03aab29b3
 Author: Laszlo Papp laszlo.p...@polatis.com
 Date:   Wed Feb 18 15:20:58 2015 +

 Delete the user from all the groups for user deletion

 diff --git a/libbb/update_passwd.c b/libbb/update_passwd.c
 index a30af6f..42998a9 100644
 --- a/libbb/update_passwd.c
 +++ b/libbb/update_passwd.c
 @@ -62,6 +62,8 @@ static void check_selinux_update_passwd(const char 
 *username)
  only if CONFIG_PASSWD=y and applet_name[0] == 'p' like in passwd
  or if CONFIG_CHPASSWD=y and applet_name[0] == 'c' like in chpasswd

 + 8) delete a user from all groups : update_passwd(FILE, NULL, NULL, MEMBER)
 +
   This function does not validate the arguments fed to it
   so the calling program should take care of that.

 @@ -81,7 +83,7 @@ int FAST_FUNC update_passwd(const char *filename,
 FILE *new_fp;
 char *fnamesfx;
 char *sfx_char;
 -   char *name_colon;
 +   char *name_colon = 0;
 unsigned user_len;
 int old_fd;
 int new_fd;
 @@ -99,13 +101,15 @@ int FAST_FUNC update_passwd(const char *filename,
 if (filename == NULL)
 return ret;

 -   check_selinux_update_passwd(name);
 +   if (name) check_selinux_update_passwd(name);

 /* New passwd file, /etc/passwd+ for now */
 fnamesfx = xasprintf(%s+, filename);
 sfx_char = fnamesfx[strlen(fnamesfx)-1];
 -   name_colon = xasprintf(%s:, name);
 -   user_len = strlen(name_colon);
 +if (name) {
 +name_colon = xasprintf(%s:, name);
 +user_len = strlen(name_colon);
 +}

 if (shadow)
 old_fp = fopen(filename, r+);
 @@ -162,21 +166,37 @@ int FAST_FUNC update_passwd(const char *filename,
 /* Read current password file, write updated /etc/passwd+ */
 changed_lines = 0;
 while (1) {
 -   char *cp, *line;
 -
 -   line = xmalloc_fgetline(old_fp);
 -   if (!line) /* EOF/error */
 -   break;
 -   if (strncmp(name_colon, line, user_len) != 0) {
 -   fprintf(new_fp, %s\n, line);
 -   goto next;
 -   }
 -
 -   /* We have a match with name:... */
 -   cp = line + user_len; /* move past name: */
 +char *cp, *line;
 +if (!name  member) {
 +struct group* g;
 +if ((g = getgrent())) {
 +char gline[LINE_MAX];
 +char **s= g-gr_mem;
 +snprintf(gline, sizeof(gline), %s:%s:%i:,
 g-gr_name, g-gr_passwd, g-gr_gid);
 +while (*s) {
 +if (strcmp(*s, member)) { if (s != g-gr_mem)
 strcat(gline, ,); strcat(gline, *s); }
 +++s;
 +}
 +fprintf(new_fp, %s\n, gline);
 +goto next;

Kaboom! That is undefined behavior because line is not initialized and
then going to next would try to free that. Line needs to be
initialized to zero either at the beginning or after free. However, I
would probably just use continue instead of goto next to carry on
without the operation of free as there is really nothing to free in
this branch. The code did work on my desktop, but not on my arm
target. This is the nature of UB, I am afraid. Once I changed this
goto to continue, it worked everywhere.

Good luck with integrating this fix into busybox. I am not willing to
modify as Denys usually goes around and changes patches anyway, so
just leaving the important notes. ;-)

 +} else {
 +break;
 +}
 +} else {
 +line = xmalloc_fgetline(old_fp);
 +if (!line) /* EOF/error */
 +break;
 +if (!name_colon || strncmp(name_colon, line, user_len) != 0) {
 +fprintf(new_fp, %s\n, line);
 +goto next;
 +}
 +
 +/* We have a match with name:... */
 +cp = line + user_len; /* move past name: */
 +}

  #if ENABLE_FEATURE_ADDUSER_TO_GROUP || ENABLE_FEATURE_DEL_USER_FROM_GROUP
 -   if (member) {
 +   if (name  member) {
 /* It's actually /etc/group+, not /etc/passwd+ */
 if (ENABLE_FEATURE_ADDUSER_TO_GROUP

Re: Bug in the deluser applet

[Laszlo Papp]

On Thu, Feb 19, 2015 at 4:56 PM, Laszlo Papp lp...@kde.org wrote:
 On Wed, Feb 18, 2015 at 3:37 PM, Laszlo Papp lp...@kde.org wrote:
 Actually, the locking and swapping logic would be common anyway which
 already resides in update_passwd, so if that reusable component is not
 moved to a separate function to be reused, we could use put our
 conditional magic into the while loop. I am preparing with that change
 now.

 Please check the patch below. I tested it and it works for me. The
 coding style may not suit the existing busybox coding style, but I do
 not mind that for now as I will use the patch as is for now in my
 local fork anyhow. Any (non-stylistic) feedback is welcome.

 commit 22a7da560af82c33ca9334039522e9a03aab29b3
 Author: Laszlo Papp laszlo.p...@polatis.com
 Date:   Wed Feb 18 15:20:58 2015 +

 Delete the user from all the groups for user deletion

 diff --git a/libbb/update_passwd.c b/libbb/update_passwd.c
 index a30af6f..42998a9 100644
 --- a/libbb/update_passwd.c
 +++ b/libbb/update_passwd.c
 @@ -62,6 +62,8 @@ static void check_selinux_update_passwd(const char 
 *username)
  only if CONFIG_PASSWD=y and applet_name[0] == 'p' like in passwd
  or if CONFIG_CHPASSWD=y and applet_name[0] == 'c' like in chpasswd

 + 8) delete a user from all groups : update_passwd(FILE, NULL, NULL, MEMBER)
 +
   This function does not validate the arguments fed to it
   so the calling program should take care of that.

 @@ -81,7 +83,7 @@ int FAST_FUNC update_passwd(const char *filename,
 FILE *new_fp;
 char *fnamesfx;
 char *sfx_char;
 -   char *name_colon;
 +   char *name_colon = 0;
 unsigned user_len;
 int old_fd;
 int new_fd;
 @@ -99,13 +101,15 @@ int FAST_FUNC update_passwd(const char *filename,
 if (filename == NULL)
 return ret;

 -   check_selinux_update_passwd(name);
 +   if (name) check_selinux_update_passwd(name);

 /* New passwd file, /etc/passwd+ for now */
 fnamesfx = xasprintf(%s+, filename);
 sfx_char = fnamesfx[strlen(fnamesfx)-1];
 -   name_colon = xasprintf(%s:, name);
 -   user_len = strlen(name_colon);
 +if (name) {
 +name_colon = xasprintf(%s:, name);
 +user_len = strlen(name_colon);
 +}

 if (shadow)
 old_fp = fopen(filename, r+);
 @@ -162,21 +166,37 @@ int FAST_FUNC update_passwd(const char *filename,
 /* Read current password file, write updated /etc/passwd+ */
 changed_lines = 0;
 while (1) {
 -   char *cp, *line;
 -
 -   line = xmalloc_fgetline(old_fp);
 -   if (!line) /* EOF/error */
 -   break;
 -   if (strncmp(name_colon, line, user_len) != 0) {
 -   fprintf(new_fp, %s\n, line);
 -   goto next;
 -   }
 -
 -   /* We have a match with name:... */
 -   cp = line + user_len; /* move past name: */
 +char *cp, *line;
 +if (!name  member) {
 +struct group* g;
 +if ((g = getgrent())) {
 +char gline[LINE_MAX];
 +char **s= g-gr_mem;
 +snprintf(gline, sizeof(gline), %s:%s:%i:,
 g-gr_name, g-gr_passwd, g-gr_gid);
 +while (*s) {
 +if (strcmp(*s, member)) { if (s != g-gr_mem)
 strcat(gline, ,); strcat(gline, *s); }

That is actually wrong, too, and needs to be fixed as soon as
possible. The issue is that if for instance the first username is to
be removed, the comma will be left in, but it should not be. I am
submitting a new change fixing this, although it is straight-forward
how to do it, in my humble opinion.

 +++s;
 +}
 +fprintf(new_fp, %s\n, gline);
 +goto next;

 Kaboom! That is undefined behavior because line is not initialized and
 then going to next would try to free that. Line needs to be
 initialized to zero either at the beginning or after free. However, I
 would probably just use continue instead of goto next to carry on
 without the operation of free as there is really nothing to free in
 this branch. The code did work on my desktop, but not on my arm
 target. This is the nature of UB, I am afraid. Once I changed this
 goto to continue, it worked everywhere.

 Good luck with integrating this fix into busybox. I am not willing to
 modify as Denys usually goes around and changes patches anyway, so
 just leaving the important notes. ;-)

 +} else {
 +break;
 +}
 +} else {
 +line = xmalloc_fgetline(old_fp);
 +if (!line) /* EOF/error */
 +break;
 +if (!name_colon || strncmp(name_colon, line, user_len) != 0) {
 +fprintf(new_fp, %s\n, line);
 +goto next;
 +}
 +
 +/* We have a match with name

Re: Bug in the deluser applet

[Laszlo Papp]

On Wed, Feb 18, 2015 at 7:34 AM, Laszlo Papp lp...@kde.org wrote:
 I thought about this, too, when writing the patch, but I rejected it because
 it is not simpler, in fact more complex, it is also not the right layer for
 the change. In addition, it would also be somewhat slower to execute.

Just to elaborate a bit more on this as I was sending that on my phone:

1) Why do you want to make a few lines more simpler to spare a few
lines to introduce bad implementation and slower operation?

2) Also, the update_passwd function already has all the structure to
handle this through the file, name and member variables. It looks like
the natural place to utilize the existing infrastructure.

3) Basically, you would go through the groups once in the embedded
list and then you would go through again rather than just doing the
thing correct in the first place at the first iteration. So basically,
you would have double embedded list iteration. Even if the group list
was stored in memory for _each_ user, it would still slightly be
slower and I would argue that wasting memory for potentially a big
file could defeat busybox's purpose in the first place. Anyway,
busybox's design goal should be to be as fast as possible. Personal
preferences should neither slow it down, nor make it use more memory
than needed to achieve the task.

 On Wednesday, February 18, 2015, tito farmat...@tiscali.it wrote:
 On Tuesday 17 February 2015 17:52:06 Laszlo Papp wrote:

 Hi,



 1) busybox addgroup user

 2) busybox adduser -G user testuser1

 3) busybox adduser -G user testuser2

 4) busybox deluser testuser1

 5) grep user /etc/passwd /etc/group



 ...

 /etc/passwd:testuser2:x:1010:1007:Linux User,,,:/home/testuser2:/bin/bash

 ...

 /etc/group:user:x:1007:testuser,testuser2

 ^^

 ...



 This is not how the desktop util behaves and I agree with the desktop

 util in this case. A non-existent user should not remain in

 /etc/group.



 Do you agree or disagree?



 Cheers, L.



 Hi,

 i agree that this case is not covered by actual bb's deluser code.

 What we cover at the moment is:



 deluser USER (and group with same name if empty)

 deluser USER from GROUP





 The desktop util behaviour could be added tough.

 By looking at the pseudo code you posted in a later

 mail I wonder if it could be easier to change

 deluser rather than update_passwd which is more complex.

 Like:



 do_deluser

 get_user_group_list

 do_delgroup of primary group

 while_list

 do_deluser_from_group

 or

 do_deluser

 while getgrent

 do_deluser_from_group(user, grname)

 do_delgroup of primary group



 Ciao,

 Tito
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Bug in the deluser applet

[Laszlo Papp]

On Wed, Feb 18, 2015 at 4:40 PM, Laszlo Papp lp...@kde.org wrote:
 On Wed, Feb 18, 2015 at 4:34 PM, tito farmat...@tiscali.it wrote:
 On Wednesday 18 February 2015 17:16:56 you wrote:

 On Wed, Feb 18, 2015 at 3:54 PM, tito farmat...@tiscali.it wrote:

  On Wednesday 18 February 2015 16:37:59 you wrote:

 

   Actually, the locking and swapping logic would be common anyway which

 

   already resides in update_passwd, so if that reusable component is
   not

 

   moved to a separate function to be reused, we could use put our

 

   conditional magic into the while loop. I am preparing with that
   change

 

   now.

 

 

 

  Please check the patch below. I tested it and it works for me. The

 

  coding style may not suit the existing busybox coding style, but I do

 

  not mind that for now as I will use the patch as is for now in my

 

  local fork anyhow. Any (non-stylistic) feedback is welcome.

 

 

 

  commit 22a7da560af82c33ca9334039522e9a03aab29b3

 

  Author: Laszlo Papp laszlo.p...@polatis.com

 

  Date: Wed Feb 18 15:20:58 2015 +

 

 

 

  Delete the user from all the groups for user deletion

 

 

 

  diff --git a/libbb/update_passwd.c b/libbb/update_passwd.c

 

  index a30af6f..42998a9 100644

 

  --- a/libbb/update_passwd.c

 

  +++ b/libbb/update_passwd.c

 

  @@ -62,6 +62,8 @@ static void check_selinux_update_passwd(const char

  *username)

 

  only if CONFIG_PASSWD=y and applet_name[0] == 'p' like in passwd

 

  or if CONFIG_CHPASSWD=y and applet_name[0] == 'c' like in chpasswd

 

 

 

  + 8) delete a user from all groups : update_passwd(FILE, NULL, NULL,

  MEMBER)

 

  +

 

  This function does not validate the arguments fed to it

 

  so the calling program should take care of that.

 

 

 

  @@ -81,7 +83,7 @@ int FAST_FUNC update_passwd(const char *filename,

 

  FILE *new_fp;

 

  char *fnamesfx;

 

  char *sfx_char;

 

  - char *name_colon;

 

  + char *name_colon = 0;

 

  unsigned user_len;

 

  int old_fd;

 

  int new_fd;

 

  @@ -99,13 +101,15 @@ int FAST_FUNC update_passwd(const char *filename,

 

  if (filename == NULL)

 

  return ret;

 

 

 

  - check_selinux_update_passwd(name);

 

  + if (name) check_selinux_update_passwd(name);

 

 

 

  /* New passwd file, /etc/passwd+ for now */

 

  fnamesfx = xasprintf(%s+, filename);

 

  sfx_char = fnamesfx[strlen(fnamesfx)-1];

 

  - name_colon = xasprintf(%s:, name);

 

  - user_len = strlen(name_colon);

 

  + if (name) {

 

  + name_colon = xasprintf(%s:, name);

 

  + user_len = strlen(name_colon);

 

  + }

 

 

 

  if (shadow)

 

  old_fp = fopen(filename, r+);

 

  @@ -162,21 +166,37 @@ int FAST_FUNC update_passwd(const char *filename,

 

  /* Read current password file, write updated /etc/passwd+ */

 

  changed_lines = 0;

 

  while (1) {

 

  - char *cp, *line;

 

  -

 

  - line = xmalloc_fgetline(old_fp);

 

  - if (!line) /* EOF/error */

 

  - break;

 

  - if (strncmp(name_colon, line, user_len) != 0) {

 

  - fprintf(new_fp, %s\n, line);

 

  - goto next;

 

  - }

 

  -

 

  - /* We have a match with name:... */

 

  - cp = line + user_len; /* move past name: */

 

  + char *cp, *line;

 

  + if (!name  member) {

 

  + struct group* g;

 

  + if ((g = getgrent())) {

 

  + char gline[LINE_MAX];

 

  + char **s= g-gr_mem;

 

  + snprintf(gline, sizeof(gline), %s:%s:%i:,

 

  g-gr_name, g-gr_passwd, g-gr_gid);

 

  + while (*s) {

 

  + if (strcmp(*s, member)) { if (s != g-gr_mem)

 

  strcat(gline, ,); strcat(gline, *s); }

 

  + ++s;

 

  + }

 

  + fprintf(new_fp, %s\n, gline);

 

  + goto next;

 

  + } else {

 

  + break;

 

  + }

 

  + } else {

 

  + line = xmalloc_fgetline(old_fp);

 

  + if (!line) /* EOF/error */

 

  + break;

 

  + if (!name_colon || strncmp(name_colon, line, user_len) != 0) {

 

  + fprintf(new_fp, %s\n, line);

 

  + goto next;

 

  + }

 

  +

 

  + /* We have a match with name:... */

 

  + cp = line + user_len; /* move past name: */

 

  + }

 

 

 

  #if ENABLE_FEATURE_ADDUSER_TO_GROUP ||
  ENABLE_FEATURE_DEL_USER_FROM_GROUP

 

  - if (member) {

 

  + if (name  member) {

 

  /* It's actually /etc/group+, not /etc/passwd+ */

 

  if (ENABLE_FEATURE_ADDUSER_TO_GROUP

 

   applet_name[0] == 'a'

 

  @@ -240,7 +260,7 @@ int FAST_FUNC update_passwd(const char *filename,

 

 

 

  if (changed_lines == 0) {

 

  #if ENABLE_FEATURE_ADDUSER_TO_GROUP ||
  ENABLE_FEATURE_DEL_USER_FROM_GROUP

 

  - if (member) {

 

  + if (name  member) {

 

  if (ENABLE_ADDGROUP  applet_name[0] == 'a')

 

  bb_error_msg(can't find %s in %s,

 

  name, filename);

 

  if (ENABLE_DELGROUP  applet_name[0] == 'd')

 

  diff --git a/loginutils/deluser.c b/loginutils/deluser.c

 

  index 01a9386..a3f5f3a 100644

 

  --- a/loginutils/deluser.c

 

  +++ b/loginutils/deluser.c

 

  @@ -82,6 +82,9 @@ int deluser_main(int argc, char **argv)

 

  do_delgroup:

 

  /* delgroup GROUP

Re: Bug in the deluser applet

[Laszlo Papp]

On Wed, Feb 18, 2015 at 3:54 PM, tito farmat...@tiscali.it wrote:
 On Wednesday 18 February 2015 16:37:59 you wrote:

  Actually, the locking and swapping logic would be common anyway which

  already resides in update_passwd, so if that reusable component is not

  moved to a separate function to be reused, we could use put our

  conditional magic into the while loop. I am preparing with that change

  now.



 Please check the patch below. I tested it and it works for me. The

 coding style may not suit the existing busybox coding style, but I do

 not mind that for now as I will use the patch as is for now in my

 local fork anyhow. Any (non-stylistic) feedback is welcome.



 commit 22a7da560af82c33ca9334039522e9a03aab29b3

 Author: Laszlo Papp laszlo.p...@polatis.com

 Date: Wed Feb 18 15:20:58 2015 +



 Delete the user from all the groups for user deletion



 diff --git a/libbb/update_passwd.c b/libbb/update_passwd.c

 index a30af6f..42998a9 100644

 --- a/libbb/update_passwd.c

 +++ b/libbb/update_passwd.c

 @@ -62,6 +62,8 @@ static void check_selinux_update_passwd(const char
 *username)

 only if CONFIG_PASSWD=y and applet_name[0] == 'p' like in passwd

 or if CONFIG_CHPASSWD=y and applet_name[0] == 'c' like in chpasswd



 + 8) delete a user from all groups : update_passwd(FILE, NULL, NULL,
 MEMBER)

 +

 This function does not validate the arguments fed to it

 so the calling program should take care of that.



 @@ -81,7 +83,7 @@ int FAST_FUNC update_passwd(const char *filename,

 FILE *new_fp;

 char *fnamesfx;

 char *sfx_char;

 - char *name_colon;

 + char *name_colon = 0;

 unsigned user_len;

 int old_fd;

 int new_fd;

 @@ -99,13 +101,15 @@ int FAST_FUNC update_passwd(const char *filename,

 if (filename == NULL)

 return ret;



 - check_selinux_update_passwd(name);

 + if (name) check_selinux_update_passwd(name);



 /* New passwd file, /etc/passwd+ for now */

 fnamesfx = xasprintf(%s+, filename);

 sfx_char = fnamesfx[strlen(fnamesfx)-1];

 - name_colon = xasprintf(%s:, name);

 - user_len = strlen(name_colon);

 + if (name) {

 + name_colon = xasprintf(%s:, name);

 + user_len = strlen(name_colon);

 + }



 if (shadow)

 old_fp = fopen(filename, r+);

 @@ -162,21 +166,37 @@ int FAST_FUNC update_passwd(const char *filename,

 /* Read current password file, write updated /etc/passwd+ */

 changed_lines = 0;

 while (1) {

 - char *cp, *line;

 -

 - line = xmalloc_fgetline(old_fp);

 - if (!line) /* EOF/error */

 - break;

 - if (strncmp(name_colon, line, user_len) != 0) {

 - fprintf(new_fp, %s\n, line);

 - goto next;

 - }

 -

 - /* We have a match with name:... */

 - cp = line + user_len; /* move past name: */

 + char *cp, *line;

 + if (!name  member) {

 + struct group* g;

 + if ((g = getgrent())) {

 + char gline[LINE_MAX];

 + char **s= g-gr_mem;

 + snprintf(gline, sizeof(gline), %s:%s:%i:,

 g-gr_name, g-gr_passwd, g-gr_gid);

 + while (*s) {

 + if (strcmp(*s, member)) { if (s != g-gr_mem)

 strcat(gline, ,); strcat(gline, *s); }

 + ++s;

 + }

 + fprintf(new_fp, %s\n, gline);

 + goto next;

 + } else {

 + break;

 + }

 + } else {

 + line = xmalloc_fgetline(old_fp);

 + if (!line) /* EOF/error */

 + break;

 + if (!name_colon || strncmp(name_colon, line, user_len) != 0) {

 + fprintf(new_fp, %s\n, line);

 + goto next;

 + }

 +

 + /* We have a match with name:... */

 + cp = line + user_len; /* move past name: */

 + }



 #if ENABLE_FEATURE_ADDUSER_TO_GROUP || ENABLE_FEATURE_DEL_USER_FROM_GROUP

 - if (member) {

 + if (name  member) {

 /* It's actually /etc/group+, not /etc/passwd+ */

 if (ENABLE_FEATURE_ADDUSER_TO_GROUP

  applet_name[0] == 'a'

 @@ -240,7 +260,7 @@ int FAST_FUNC update_passwd(const char *filename,



 if (changed_lines == 0) {

 #if ENABLE_FEATURE_ADDUSER_TO_GROUP || ENABLE_FEATURE_DEL_USER_FROM_GROUP

 - if (member) {

 + if (name  member) {

 if (ENABLE_ADDGROUP  applet_name[0] == 'a')

 bb_error_msg(can't find %s in %s,

 name, filename);

 if (ENABLE_DELGROUP  applet_name[0] == 'd')

 diff --git a/loginutils/deluser.c b/loginutils/deluser.c

 index 01a9386..a3f5f3a 100644

 --- a/loginutils/deluser.c

 +++ b/loginutils/deluser.c

 @@ -82,6 +82,9 @@ int deluser_main(int argc, char **argv)

 do_delgroup:

 /* delgroup GROUP or delgroup USER GROUP */

 if (do_deluser  0) { /* delgroup after deluser? */

 + pfile = bb_path_group_file;

 + if (update_passwd(pfile, NULL, NULL, name) == -1)

 + return EXIT_FAILURE;

 gr = getgrnam(name);

 if (!gr)

 return EXIT_SUCCESS;

 @@ -99,7 +102,6 @@ int deluser_main(int argc, char **argv)

 }

 //endpwent();

 }

 - pfile = bb_path_group_file;

 if (ENABLE_FEATURE_SHADOWPASSWDS)

 sfile = bb_path_gshadow_file;

 }





 Hi,

 just for fun a different approach, this seems to work

 without touching update_passwd.

The real question is: why would you not touch that? All the locking
and swapping are done in there properly and that is the fastest way to
do it because you avoid the needless

Re: Bug in the deluser applet

[Laszlo Papp]

On Wed, Feb 18, 2015 at 12:09 PM, Laszlo Papp lp...@kde.org wrote:
 On Wed, Feb 18, 2015 at 11:00 AM, tito farmat...@tiscali.it wrote:
 On Wednesday 18 February 2015 09:14:42 you wrote:

 On Wed, Feb 18, 2015 at 8:09 AM, Laszlo Papp lp...@kde.org wrote:

  On Wed, Feb 18, 2015 at 7:34 AM, Laszlo Papp lp...@kde.org wrote:

  I thought about this, too, when writing the patch, but I rejected it
  because

  it is not simpler, in fact more complex, it is also not the right layer
  for

  the change. In addition, it would also be somewhat slower to execute.

 

  Just to elaborate a bit more on this as I was sending that on my phone:

 

  1) Why do you want to make a few lines more simpler to spare a few

  lines to introduce bad implementation and slower operation?

 

  2) Also, the update_passwd function already has all the structure to

  handle this through the file, name and member variables. It looks like

  the natural place to utilize the existing infrastructure.

 

  3) Basically, you would go through the groups once in the embedded

  list and then you would go through again rather than just doing the

  thing correct in the first place at the first iteration. So basically,

  you would have double embedded list iteration. Even if the group list

  was stored in memory for _each_ user, it would still slightly be

  slower and I would argue that wasting memory for potentially a big

  file could defeat busybox's purpose in the first place. Anyway,

  busybox's design goal should be to be as fast as possible. Personal

  preferences should neither slow it down, nor make it use more memory

  than needed to achieve the task.



 I would actually even go further than that, namely: the same-named

 group deletion should probably be integrated into my loop so that one

 embedded iteration could deal with the group file changes rather than

 two separate.





 Hi,



 you can try to write a patch, but i suspect that it could be difficult

 with the current update_password interface without breaking the

 deluser from group use case.

 If you opt for doing the changes in deluser instead some useful

 get_groups code is in the id applet and could be moved to libbb as

 function. This will of course be slower as more iterations are done

 through the passwd/group files.

 I start to think that the best approach would be neither to use your
 idea, nor mine, but actually a separate function doing the thing fast.
 See my updated patch below which almost works, but it remains slow if
 I try to inject the new code into the line based read and write
 concept. Why don't we just create a new function?

Actually, the locking and swapping logic would be common anyway which
already resides in update_passwd, so if that reusable component is not
moved to a separate function to be reused, we could use put our
conditional magic into the while loop. I am preparing with that change
now.

By the way, why does busybox use /etc/file+ and /etc/file- for this
logic as opposed to ... say: mkstemp?

 commit 813e3db073aca7a55b9f3d4650055e7d1ed768b4
 Author: Laszlo Papp lp...@kde.org
 Date:   Tue Feb 17 20:01:04 2015 +

 Delete the user from all the groups for user deletion

 diff --git a/libbb/update_passwd.c b/libbb/update_passwd.c
 index a30af6f..24f949b 100644
 --- a/libbb/update_passwd.c
 +++ b/libbb/update_passwd.c
 @@ -62,6 +62,8 @@ static void check_selinux_update_passwd(const char 
 *username)
  only if CONFIG_PASSWD=y and applet_name[0] == 'p' like in passwd
  or if CONFIG_CHPASSWD=y and applet_name[0] == 'c' like in chpasswd

 + 8) delete a user from all groups : update_passwd(FILE, NULL, NULL, MEMBER)
 +
   This function does not validate the arguments fed to it
   so the calling program should take care of that.

 @@ -81,7 +83,7 @@ int FAST_FUNC update_passwd(const char *filename,
 FILE *new_fp;
 char *fnamesfx;
 char *sfx_char;
 -   char *name_colon;
 +   char *name_colon = 0;
 unsigned user_len;
 int old_fd;
 int new_fd;
 @@ -99,13 +101,15 @@ int FAST_FUNC update_passwd(const char *filename,
 if (filename == NULL)
 return ret;

 -   check_selinux_update_passwd(name);
 +   if (name) check_selinux_update_passwd(name);

 /* New passwd file, /etc/passwd+ for now */
 fnamesfx = xasprintf(%s+, filename);
 sfx_char = fnamesfx[strlen(fnamesfx)-1];
 -   name_colon = xasprintf(%s:, name);
 -   user_len = strlen(name_colon);
 +if (name) {
 +name_colon = xasprintf(%s:, name);
 +user_len = strlen(name_colon);
 +}

 if (shadow)
 old_fp = fopen(filename, r+);
 @@ -159,6 +163,19 @@ int FAST_FUNC update_passwd(const char *filename,
 bb_perror_msg(warning: can't lock '%s', filename);
 lock.l_type = F_UNLCK;

 +if (!name  member) {
 +struct group* g;
 +while ((g = getgrent())) {
 +char **s= g-gr_mem

Re: Bug in the deluser applet

[Laszlo Papp]

 Actually, the locking and swapping logic would be common anyway which
 already resides in update_passwd, so if that reusable component is not
 moved to a separate function to be reused, we could use put our
 conditional magic into the while loop. I am preparing with that change
 now.

Please check the patch below. I tested it and it works for me. The
coding style may not suit the existing busybox coding style, but I do
not mind that for now as I will use the patch as is for now in my
local fork anyhow. Any (non-stylistic) feedback is welcome.

commit 22a7da560af82c33ca9334039522e9a03aab29b3
Author: Laszlo Papp laszlo.p...@polatis.com
Date:   Wed Feb 18 15:20:58 2015 +

Delete the user from all the groups for user deletion

diff --git a/libbb/update_passwd.c b/libbb/update_passwd.c
index a30af6f..42998a9 100644
--- a/libbb/update_passwd.c
+++ b/libbb/update_passwd.c
@@ -62,6 +62,8 @@ static void check_selinux_update_passwd(const char *username)
 only if CONFIG_PASSWD=y and applet_name[0] == 'p' like in passwd
 or if CONFIG_CHPASSWD=y and applet_name[0] == 'c' like in chpasswd

+ 8) delete a user from all groups : update_passwd(FILE, NULL, NULL, MEMBER)
+
  This function does not validate the arguments fed to it
  so the calling program should take care of that.

@@ -81,7 +83,7 @@ int FAST_FUNC update_passwd(const char *filename,
FILE *new_fp;
char *fnamesfx;
char *sfx_char;
-   char *name_colon;
+   char *name_colon = 0;
unsigned user_len;
int old_fd;
int new_fd;
@@ -99,13 +101,15 @@ int FAST_FUNC update_passwd(const char *filename,
if (filename == NULL)
return ret;

-   check_selinux_update_passwd(name);
+   if (name) check_selinux_update_passwd(name);

/* New passwd file, /etc/passwd+ for now */
fnamesfx = xasprintf(%s+, filename);
sfx_char = fnamesfx[strlen(fnamesfx)-1];
-   name_colon = xasprintf(%s:, name);
-   user_len = strlen(name_colon);
+if (name) {
+name_colon = xasprintf(%s:, name);
+user_len = strlen(name_colon);
+}

if (shadow)
old_fp = fopen(filename, r+);
@@ -162,21 +166,37 @@ int FAST_FUNC update_passwd(const char *filename,
/* Read current password file, write updated /etc/passwd+ */
changed_lines = 0;
while (1) {
-   char *cp, *line;
-
-   line = xmalloc_fgetline(old_fp);
-   if (!line) /* EOF/error */
-   break;
-   if (strncmp(name_colon, line, user_len) != 0) {
-   fprintf(new_fp, %s\n, line);
-   goto next;
-   }
-
-   /* We have a match with name:... */
-   cp = line + user_len; /* move past name: */
+char *cp, *line;
+if (!name  member) {
+struct group* g;
+if ((g = getgrent())) {
+char gline[LINE_MAX];
+char **s= g-gr_mem;
+snprintf(gline, sizeof(gline), %s:%s:%i:,
g-gr_name, g-gr_passwd, g-gr_gid);
+while (*s) {
+if (strcmp(*s, member)) { if (s != g-gr_mem)
strcat(gline, ,); strcat(gline, *s); }
+++s;
+}
+fprintf(new_fp, %s\n, gline);
+goto next;
+} else {
+break;
+}
+} else {
+line = xmalloc_fgetline(old_fp);
+if (!line) /* EOF/error */
+break;
+if (!name_colon || strncmp(name_colon, line, user_len) != 0) {
+fprintf(new_fp, %s\n, line);
+goto next;
+}
+
+/* We have a match with name:... */
+cp = line + user_len; /* move past name: */
+}

 #if ENABLE_FEATURE_ADDUSER_TO_GROUP || ENABLE_FEATURE_DEL_USER_FROM_GROUP
-   if (member) {
+   if (name  member) {
/* It's actually /etc/group+, not /etc/passwd+ */
if (ENABLE_FEATURE_ADDUSER_TO_GROUP
  applet_name[0] == 'a'
@@ -240,7 +260,7 @@ int FAST_FUNC update_passwd(const char *filename,

if (changed_lines == 0) {
 #if ENABLE_FEATURE_ADDUSER_TO_GROUP || ENABLE_FEATURE_DEL_USER_FROM_GROUP
-   if (member) {
+   if (name  member) {
if (ENABLE_ADDGROUP  applet_name[0] == 'a')
bb_error_msg(can't find %s in %s,
name, filename);
if (ENABLE_DELGROUP  applet_name[0] == 'd')
diff --git a/loginutils/deluser.c b/loginutils/deluser.c
index 01a9386..a3f5f3a 100644
--- a/loginutils/deluser.c
+++ b/loginutils/deluser.c
@@ -82,6 +82,9 @@ int deluser_main(int argc, char **argv)
  do_delgroup:
/* delgroup GROUP or delgroup USER GROUP */
if (do_deluser  0) { /* delgroup after deluser

Re: Bug in the deluser applet

[Laszlo Papp]

On Wed, Feb 18, 2015 at 11:00 AM, tito farmat...@tiscali.it wrote:
 On Wednesday 18 February 2015 09:14:42 you wrote:

 On Wed, Feb 18, 2015 at 8:09 AM, Laszlo Papp lp...@kde.org wrote:

  On Wed, Feb 18, 2015 at 7:34 AM, Laszlo Papp lp...@kde.org wrote:

  I thought about this, too, when writing the patch, but I rejected it
  because

  it is not simpler, in fact more complex, it is also not the right layer
  for

  the change. In addition, it would also be somewhat slower to execute.

 

  Just to elaborate a bit more on this as I was sending that on my phone:

 

  1) Why do you want to make a few lines more simpler to spare a few

  lines to introduce bad implementation and slower operation?

 

  2) Also, the update_passwd function already has all the structure to

  handle this through the file, name and member variables. It looks like

  the natural place to utilize the existing infrastructure.

 

  3) Basically, you would go through the groups once in the embedded

  list and then you would go through again rather than just doing the

  thing correct in the first place at the first iteration. So basically,

  you would have double embedded list iteration. Even if the group list

  was stored in memory for _each_ user, it would still slightly be

  slower and I would argue that wasting memory for potentially a big

  file could defeat busybox's purpose in the first place. Anyway,

  busybox's design goal should be to be as fast as possible. Personal

  preferences should neither slow it down, nor make it use more memory

  than needed to achieve the task.



 I would actually even go further than that, namely: the same-named

 group deletion should probably be integrated into my loop so that one

 embedded iteration could deal with the group file changes rather than

 two separate.





 Hi,



 you can try to write a patch, but i suspect that it could be difficult

 with the current update_password interface without breaking the

 deluser from group use case.

 If you opt for doing the changes in deluser instead some useful

 get_groups code is in the id applet and could be moved to libbb as

 function. This will of course be slower as more iterations are done

 through the passwd/group files.

I start to think that the best approach would be neither to use your
idea, nor mine, but actually a separate function doing the thing fast.
See my updated patch below which almost works, but it remains slow if
I try to inject the new code into the line based read and write
concept. Why don't we just create a new function?

commit 813e3db073aca7a55b9f3d4650055e7d1ed768b4
Author: Laszlo Papp lp...@kde.org
Date:   Tue Feb 17 20:01:04 2015 +

Delete the user from all the groups for user deletion

diff --git a/libbb/update_passwd.c b/libbb/update_passwd.c
index a30af6f..24f949b 100644
--- a/libbb/update_passwd.c
+++ b/libbb/update_passwd.c
@@ -62,6 +62,8 @@ static void check_selinux_update_passwd(const char *username)
 only if CONFIG_PASSWD=y and applet_name[0] == 'p' like in passwd
 or if CONFIG_CHPASSWD=y and applet_name[0] == 'c' like in chpasswd

+ 8) delete a user from all groups : update_passwd(FILE, NULL, NULL, MEMBER)
+
  This function does not validate the arguments fed to it
  so the calling program should take care of that.

@@ -81,7 +83,7 @@ int FAST_FUNC update_passwd(const char *filename,
FILE *new_fp;
char *fnamesfx;
char *sfx_char;
-   char *name_colon;
+   char *name_colon = 0;
unsigned user_len;
int old_fd;
int new_fd;
@@ -99,13 +101,15 @@ int FAST_FUNC update_passwd(const char *filename,
if (filename == NULL)
return ret;

-   check_selinux_update_passwd(name);
+   if (name) check_selinux_update_passwd(name);

/* New passwd file, /etc/passwd+ for now */
fnamesfx = xasprintf(%s+, filename);
sfx_char = fnamesfx[strlen(fnamesfx)-1];
-   name_colon = xasprintf(%s:, name);
-   user_len = strlen(name_colon);
+if (name) {
+name_colon = xasprintf(%s:, name);
+user_len = strlen(name_colon);
+}

if (shadow)
old_fp = fopen(filename, r+);
@@ -159,6 +163,19 @@ int FAST_FUNC update_passwd(const char *filename,
bb_perror_msg(warning: can't lock '%s', filename);
lock.l_type = F_UNLCK;

+if (!name  member) {
+struct group* g;
+while ((g = getgrent())) {
+char **s= g-gr_mem;
+char **d = s;
+while (*s) {
+if (strcmp(*s, member)) { *d = *s; ++d; }
+++s;
+}
+*d = 0;
+}
+}
+
/* Read current password file, write updated /etc/passwd+ */
changed_lines = 0;
while (1) {
@@ -167,7 +184,7 @@ int FAST_FUNC update_passwd(const char *filename,
line = xmalloc_fgetline(old_fp);
if (!line) /* EOF/error */
break

Re: Bug in the deluser applet

[Laszlo Papp]

On Wed, Feb 18, 2015 at 3:08 PM, tito farmat...@tiscali.it wrote:
 On Wednesday 18 February 2015 15:13:21 you wrote:

 On Wed, Feb 18, 2015 at 12:09 PM, Laszlo Papp lp...@kde.org wrote:

  On Wed, Feb 18, 2015 at 11:00 AM, tito farmat...@tiscali.it wrote:

  On Wednesday 18 February 2015 09:14:42 you wrote:

 

  On Wed, Feb 18, 2015 at 8:09 AM, Laszlo Papp lp...@kde.org wrote:

 

   On Wed, Feb 18, 2015 at 7:34 AM, Laszlo Papp lp...@kde.org wrote:

 

   I thought about this, too, when writing the patch, but I rejected
   it

   because

 

   it is not simpler, in fact more complex, it is also not the right
   layer

   for

 

   the change. In addition, it would also be somewhat slower to
   execute.

 

  

 

   Just to elaborate a bit more on this as I was sending that on my
   phone:

 

  

 

   1) Why do you want to make a few lines more simpler to spare a few

 

   lines to introduce bad implementation and slower operation?

 

  

 

   2) Also, the update_passwd function already has all the structure to

 

   handle this through the file, name and member variables. It looks
   like

 

   the natural place to utilize the existing infrastructure.

 

  

 

   3) Basically, you would go through the groups once in the embedded

 

   list and then you would go through again rather than just doing the

 

   thing correct in the first place at the first iteration. So
   basically,

 

   you would have double embedded list iteration. Even if the group
   list

 

   was stored in memory for _each_ user, it would still slightly be

 

   slower and I would argue that wasting memory for potentially a big

 

   file could defeat busybox's purpose in the first place. Anyway,

 

   busybox's design goal should be to be as fast as possible. Personal

 

   preferences should neither slow it down, nor make it use more memory

 

   than needed to achieve the task.

 

 

 

  I would actually even go further than that, namely: the same-named

 

  group deletion should probably be integrated into my loop so that one

 

  embedded iteration could deal with the group file changes rather than

 

  two separate.

 

 

 

 

 

  Hi,

 

 

 

  you can try to write a patch, but i suspect that it could be difficult

 

  with the current update_password interface without breaking the

 

  deluser from group use case.

 

  If you opt for doing the changes in deluser instead some useful

 

  get_groups code is in the id applet and could be moved to libbb as

 

  function. This will of course be slower as more iterations are done

 

  through the passwd/group files.

 

  I start to think that the best approach would be neither to use your

  idea, nor mine, but actually a separate function doing the thing fast.

  See my updated patch below which almost works, but it remains slow if

  I try to inject the new code into the line based read and write

  concept. Why don't we just create a new function?



 Actually, the locking and swapping logic would be common anyway which

 already resides in update_passwd, so if that reusable component is not

 moved to a separate function to be reused, we could use put our

 conditional magic into the while loop. I am preparing with that change

 now.



 By the way, why does busybox use /etc/file+ and /etc/file- for this

 logic as opposed to ... say: mkstemp?



 So other instances of programs that want to change

 file know that it is in use (file+ existing) and wait?

Is that a valid reasoning?

The file ought to be opened in locked mode all the time when starting
the operation, in my opinion, in which case this does not apply.
Furthermore, if I precreate the lock file like that, it is easy to
break the applet by just creating the file beforehand.

man 2 flock (LOCK_EX)
man 3 mkstemp




  commit 813e3db073aca7a55b9f3d4650055e7d1ed768b4

  Author: Laszlo Papp lp...@kde.org

  Date: Tue Feb 17 20:01:04 2015 +

 

  Delete the user from all the groups for user deletion

 

  diff --git a/libbb/update_passwd.c b/libbb/update_passwd.c

  index a30af6f..24f949b 100644

  --- a/libbb/update_passwd.c

  +++ b/libbb/update_passwd.c

  @@ -62,6 +62,8 @@ static void check_selinux_update_passwd(const char
  *username)

  only if CONFIG_PASSWD=y and applet_name[0] == 'p' like in passwd

  or if CONFIG_CHPASSWD=y and applet_name[0] == 'c' like in chpasswd

 

  + 8) delete a user from all groups : update_passwd(FILE, NULL, NULL,
  MEMBER)

  +

  This function does not validate the arguments fed to it

  so the calling program should take care of that.

 

  @@ -81,7 +83,7 @@ int FAST_FUNC update_passwd(const char *filename,

  FILE *new_fp;

  char *fnamesfx;

  char *sfx_char;

  - char *name_colon;

  + char *name_colon = 0;

  unsigned user_len;

  int old_fd;

  int new_fd;

  @@ -99,13 +101,15 @@ int FAST_FUNC update_passwd(const char *filename,

  if (filename == NULL)

  return ret;

 

  - check_selinux_update_passwd(name);

  + if (name

Re: Bug in the deluser applet

[Laszlo Papp]

On Wed, Feb 18, 2015 at 4:34 PM, tito farmat...@tiscali.it wrote:
 On Wednesday 18 February 2015 17:16:56 you wrote:

 On Wed, Feb 18, 2015 at 3:54 PM, tito farmat...@tiscali.it wrote:

  On Wednesday 18 February 2015 16:37:59 you wrote:

 

   Actually, the locking and swapping logic would be common anyway which

 

   already resides in update_passwd, so if that reusable component is
   not

 

   moved to a separate function to be reused, we could use put our

 

   conditional magic into the while loop. I am preparing with that
   change

 

   now.

 

 

 

  Please check the patch below. I tested it and it works for me. The

 

  coding style may not suit the existing busybox coding style, but I do

 

  not mind that for now as I will use the patch as is for now in my

 

  local fork anyhow. Any (non-stylistic) feedback is welcome.

 

 

 

  commit 22a7da560af82c33ca9334039522e9a03aab29b3

 

  Author: Laszlo Papp laszlo.p...@polatis.com

 

  Date: Wed Feb 18 15:20:58 2015 +

 

 

 

  Delete the user from all the groups for user deletion

 

 

 

  diff --git a/libbb/update_passwd.c b/libbb/update_passwd.c

 

  index a30af6f..42998a9 100644

 

  --- a/libbb/update_passwd.c

 

  +++ b/libbb/update_passwd.c

 

  @@ -62,6 +62,8 @@ static void check_selinux_update_passwd(const char

  *username)

 

  only if CONFIG_PASSWD=y and applet_name[0] == 'p' like in passwd

 

  or if CONFIG_CHPASSWD=y and applet_name[0] == 'c' like in chpasswd

 

 

 

  + 8) delete a user from all groups : update_passwd(FILE, NULL, NULL,

  MEMBER)

 

  +

 

  This function does not validate the arguments fed to it

 

  so the calling program should take care of that.

 

 

 

  @@ -81,7 +83,7 @@ int FAST_FUNC update_passwd(const char *filename,

 

  FILE *new_fp;

 

  char *fnamesfx;

 

  char *sfx_char;

 

  - char *name_colon;

 

  + char *name_colon = 0;

 

  unsigned user_len;

 

  int old_fd;

 

  int new_fd;

 

  @@ -99,13 +101,15 @@ int FAST_FUNC update_passwd(const char *filename,

 

  if (filename == NULL)

 

  return ret;

 

 

 

  - check_selinux_update_passwd(name);

 

  + if (name) check_selinux_update_passwd(name);

 

 

 

  /* New passwd file, /etc/passwd+ for now */

 

  fnamesfx = xasprintf(%s+, filename);

 

  sfx_char = fnamesfx[strlen(fnamesfx)-1];

 

  - name_colon = xasprintf(%s:, name);

 

  - user_len = strlen(name_colon);

 

  + if (name) {

 

  + name_colon = xasprintf(%s:, name);

 

  + user_len = strlen(name_colon);

 

  + }

 

 

 

  if (shadow)

 

  old_fp = fopen(filename, r+);

 

  @@ -162,21 +166,37 @@ int FAST_FUNC update_passwd(const char *filename,

 

  /* Read current password file, write updated /etc/passwd+ */

 

  changed_lines = 0;

 

  while (1) {

 

  - char *cp, *line;

 

  -

 

  - line = xmalloc_fgetline(old_fp);

 

  - if (!line) /* EOF/error */

 

  - break;

 

  - if (strncmp(name_colon, line, user_len) != 0) {

 

  - fprintf(new_fp, %s\n, line);

 

  - goto next;

 

  - }

 

  -

 

  - /* We have a match with name:... */

 

  - cp = line + user_len; /* move past name: */

 

  + char *cp, *line;

 

  + if (!name  member) {

 

  + struct group* g;

 

  + if ((g = getgrent())) {

 

  + char gline[LINE_MAX];

 

  + char **s= g-gr_mem;

 

  + snprintf(gline, sizeof(gline), %s:%s:%i:,

 

  g-gr_name, g-gr_passwd, g-gr_gid);

 

  + while (*s) {

 

  + if (strcmp(*s, member)) { if (s != g-gr_mem)

 

  strcat(gline, ,); strcat(gline, *s); }

 

  + ++s;

 

  + }

 

  + fprintf(new_fp, %s\n, gline);

 

  + goto next;

 

  + } else {

 

  + break;

 

  + }

 

  + } else {

 

  + line = xmalloc_fgetline(old_fp);

 

  + if (!line) /* EOF/error */

 

  + break;

 

  + if (!name_colon || strncmp(name_colon, line, user_len) != 0) {

 

  + fprintf(new_fp, %s\n, line);

 

  + goto next;

 

  + }

 

  +

 

  + /* We have a match with name:... */

 

  + cp = line + user_len; /* move past name: */

 

  + }

 

 

 

  #if ENABLE_FEATURE_ADDUSER_TO_GROUP ||
  ENABLE_FEATURE_DEL_USER_FROM_GROUP

 

  - if (member) {

 

  + if (name  member) {

 

  /* It's actually /etc/group+, not /etc/passwd+ */

 

  if (ENABLE_FEATURE_ADDUSER_TO_GROUP

 

   applet_name[0] == 'a'

 

  @@ -240,7 +260,7 @@ int FAST_FUNC update_passwd(const char *filename,

 

 

 

  if (changed_lines == 0) {

 

  #if ENABLE_FEATURE_ADDUSER_TO_GROUP ||
  ENABLE_FEATURE_DEL_USER_FROM_GROUP

 

  - if (member) {

 

  + if (name  member) {

 

  if (ENABLE_ADDGROUP  applet_name[0] == 'a')

 

  bb_error_msg(can't find %s in %s,

 

  name, filename);

 

  if (ENABLE_DELGROUP  applet_name[0] == 'd')

 

  diff --git a/loginutils/deluser.c b/loginutils/deluser.c

 

  index 01a9386..a3f5f3a 100644

 

  --- a/loginutils/deluser.c

 

  +++ b/loginutils/deluser.c

 

  @@ -82,6 +82,9 @@ int deluser_main(int argc, char **argv)

 

  do_delgroup:

 

  /* delgroup GROUP or delgroup USER GROUP */

 

  if (do_deluser  0) { /* delgroup after

Bug in the deluser applet

[Laszlo Papp]

Hi,

1) busybox addgroup user
2) busybox adduser -G user testuser1
3) busybox adduser -G user testuser2
4) busybox deluser testuser1
5) grep user /etc/passwd /etc/group

...
/etc/passwd:testuser2:x:1010:1007:Linux User,,,:/home/testuser2:/bin/bash
...
/etc/group:user:x:1007:testuser,testuser2
   ^^
...

This is not how the desktop util behaves and I agree with the desktop
util in this case. A non-existent user should not remain in
/etc/group.

Do you agree or disagree?

Cheers, L.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Bug in the deluser applet

[Laszlo Papp]

On Tue, Feb 17, 2015 at 4:52 PM, Laszlo Papp lp...@kde.org wrote:
 Hi,

 1) busybox addgroup user
 2) busybox adduser -G user testuser1
 3) busybox adduser -G user testuser2
 4) busybox deluser testuser1
 5) grep user /etc/passwd /etc/group

 ...
 /etc/passwd:testuser2:x:1010:1007:Linux User,,,:/home/testuser2:/bin/bash
 ...
 /etc/group:user:x:1007:testuser,testuser2
^^

Apologies; that is wrong copy/paste. I meant to write this:

/etc/group:user:x:1007:testuser1,testuser2
^^
 ...

 This is not how the desktop util behaves and I agree with the desktop
 util in this case. A non-existent user should not remain in
 /etc/group.

 Do you agree or disagree?

 Cheers, L.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Bug in the deluser applet

[Laszlo Papp]

WARNING: this is pseudo code. Do not use it as is. It makes my
/etc/group empty after the run, but it ought to show the concept I am
thinking of. As indicated, the implementation has some bug currently.
Any feedback is welcome.

commit d7075bb9195a65d11b43f516112f5b0134cbc62a
Author: Laszlo Papp laszlo.p...@polatis.com
Date:   Tue Feb 17 20:01:04 2015 +

Delete the user from all the groups for user deletion

diff --git a/libbb/update_passwd.c b/libbb/update_passwd.c
index a30af6f..f280813 100644
--- a/libbb/update_passwd.c
+++ b/libbb/update_passwd.c
@@ -62,6 +62,8 @@ static void check_selinux_update_passwd(const char *username)
 only if CONFIG_PASSWD=y and applet_name[0] == 'p' like in passwd
 or if CONFIG_CHPASSWD=y and applet_name[0] == 'c' like in chpasswd

+ 8) delete a user from all groups : update_passwd(FILE, NULL, NULL, MEMBER)
+
  This function does not validate the arguments fed to it
  so the calling program should take care of that.

@@ -81,7 +83,7 @@ int FAST_FUNC update_passwd(const char *filename,
FILE *new_fp;
char *fnamesfx;
char *sfx_char;
-   char *name_colon;
+   char *name_colon = 0;
unsigned user_len;
int old_fd;
int new_fd;
@@ -99,13 +101,15 @@ int FAST_FUNC update_passwd(const char *filename,
if (filename == NULL)
return ret;

-   check_selinux_update_passwd(name);
+   if (name) check_selinux_update_passwd(name);

/* New passwd file, /etc/passwd+ for now */
fnamesfx = xasprintf(%s+, filename);
sfx_char = fnamesfx[strlen(fnamesfx)-1];
-   name_colon = xasprintf(%s:, name);
-   user_len = strlen(name_colon);
+if (name) {
+name_colon = xasprintf(%s:, name);
+user_len = strlen(name_colon);
+}

if (shadow)
old_fp = fopen(filename, r+);
@@ -167,7 +171,7 @@ int FAST_FUNC update_passwd(const char *filename,
line = xmalloc_fgetline(old_fp);
if (!line) /* EOF/error */
break;
-   if (strncmp(name_colon, line, user_len) != 0) {
+   if (name  strncmp(name_colon, line, user_len) != 0) {
fprintf(new_fp, %s\n, line);
goto next;
}
@@ -175,8 +179,21 @@ int FAST_FUNC update_passwd(const char *filename,
/* We have a match with name:... */
cp = line + user_len; /* move past name: */

+if (!name  member) {
+struct group* g;
+while ((g = getgrent())) {
+char **s= g-gr_mem;
+char **d = s;
+while (s  *s) {
+if (strcmp(*s, member)) { *d = *s; ++d; }
+++s;
+}
+*d = 0;
+}
+}
+
 #if ENABLE_FEATURE_ADDUSER_TO_GROUP || ENABLE_FEATURE_DEL_USER_FROM_GROUP
-   if (member) {
+if (name  member) {
/* It's actually /etc/group+, not /etc/passwd+ */
if (ENABLE_FEATURE_ADDUSER_TO_GROUP
  applet_name[0] == 'a'
diff --git a/loginutils/deluser.c b/loginutils/deluser.c
index 01a9386..8219569 100644
--- a/loginutils/deluser.c
+++ b/loginutils/deluser.c
@@ -82,6 +82,9 @@ int deluser_main(int argc, char **argv)
  do_delgroup:
/* delgroup GROUP or delgroup USER GROUP */
if (do_deluser  0) { /* delgroup after deluser? */
+   pfile = bb_path_group_file;
+if (update_passwd(pfile, NULL, NULL, name) == -1)
+return EXIT_FAILURE;
gr = getgrnam(name);
if (!gr)
return EXIT_SUCCESS;
@@ -99,7 +102,6 @@ int deluser_main(int argc, char **argv)
}
//endpwent();
}
-   pfile = bb_path_group_file;
if (ENABLE_FEATURE_SHADOWPASSWDS)
sfile = bb_path_gshadow_file;
}
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Changing the primary group

[Laszlo Papp]

Hi,

I am looking for something like usermod -g on desktop. Is it possible
to achieve this feature?

There are workarounds like adding a group and then removing the
previous, etc, but it is suboptimal.

Cheers, L.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Changing the primary group

[Laszlo Papp]

On Tue, Feb 17, 2015 at 12:38 PM, walter harms wha...@bfs.de wrote:


 Am 17.02.2015 12:59, schrieb Laszlo Papp:
 Hi,

 I am looking for something like usermod -g on desktop. Is it possible
 to achieve this feature?

 I have no idea why you need usermod -g (i assume adding a group)
 i use
 vi /etc/group

1) If you have no idea, you can read the email subject: Changing the
primary group

2) Primary groups ends up /etc/passwd, not /etc/group.
Correspondingly, it is the wrong place to look at.

 There are workarounds like adding a group and then removing the
 previous, etc, but it is suboptimal.

 mmmg, changing the primary group is changing the id in the 4th field.
 see: man 5 passwd

I know, but that is not the point of the question.

1) Editing files manually are bad habit without avoiding race, etc. It
is also error-prone. You could say the same for pretty much any passwd
and group related issues that you can add a group manually.

2) It is not enough to just look up man 5 passwd. Eventually, you
would need to get the group id out of the groupname. I definitely do
not want to supply raw group ids. That is mad. :-)

Since it was not clear, I will try to clarify: is this feature already
available in busybox via one-pass? My suggested workaround is still
nicer than editing files manually like that in my opinion, but it is
two-pass.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Deluser: deleting the home folder

[Laszlo Papp]

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

[Laszlo Papp]

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

[Laszlo Papp]

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: Can OpenWrt code be used?

[Laszlo Papp]

On Sun, Jan 18, 2015 at 9:27 PM, tito farmat...@tiscali.it wrote:
 On Sunday 18 January 2015 20:30:53 m...@grounded.net wrote:
 I have been looking for a way to add curl and mtr into BusyBox for windows 
 but cannot find anything anywhere.

 Was wondering if someone might know if those tools from the OpenWrt project 
 could be re-compiled to work with the win based BusyBox?

 Thanks.
 Hi,
 I think yes if the code is portable to windows and you compile them the same 
 way you
 did with busybox.
 You could also take a look to the Cygwin project https://www.cygwin.com/
 and the mingw and mysys projects at http://www.mingw.org/.

I would rather not recommend those. They are old things. Have a look
at these instead:

http://mingw-w64.sourceforge.net/
http://msys2.sourceforge.net/

MSYS2 already has a busybox package, for instance.


 Ciao,
 Tito
 ___
 busybox mailing list
 busybox@busybox.net
 http://lists.busybox.net/mailman/listinfo/busybox
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] malloced getpw/grxxx functions for bb

[Laszlo Papp]

On Sat, Jan 3, 2015 at 12:38 AM, tito farmat...@tiscali.it wrote:
 On Friday 02 January 2015 21:04:04 you wrote:

 On Mon, Dec 29, 2014 at 10:19 PM, tito farmat...@tiscali.it wrote:

  after putting more work at this malloced libpwdgrp functions I think
  they now are

  pretty robust, with no memory leaks and able to handle more or less
  gracefully

  problematic passwd/group/shadow files. The main features of this

  implemantation 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 so far in the

  * passwd/group files and reused for later calls.

  * If ENABLE_FEATURE_CLEAN_UP is set the buffers are freed at program

  * exit using the atexit function to make valgrind happy.

  * 2) the passwd/group files:

  * a) must contain the expected number of fields (as per count of field

  * delimeters :) or we will complain with a error message.

  * b) leading or trailing whitespace in fields is allowed and handled.

  * c) some fields are not allowed to be empty (e.g. username, uid/gid,

  * homedir, shell) and in this case NULL is returned and errno is

  * set to EINVAL. This behaviour could be easily changed by

  * modifying PW_DEF, GR_DEF, SP_DEF strings (uppercase

  * makes a field mandatory).

  * d) the string representing uid/gid must be convertible by strtoXX

  * functions or NULL is returned and errno is set to EINVAL.

  * e) leading or trailing whitespaces in member names and empty members

  * are allowed and handled.

  * 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.

 

  I've done some testing and everything seems to work as expected

  nonetheless some more testing by the list members and a good

  code review by more experienced programmers is needed as this

  lib calls stuff is very new for me and maybe I overlooked

  something obvious.

  Attached you will find a patch and a drop in replacement

  for pwd_grp.c for easier review and testing.

 

  Size increase is about 120 bytes due to the increased feature set,

  but the bloat-o-meter output looks somewhat suspicious:

 

  ./scripts/bloat-o-meter busybox_unstripped_original busybox_unstripped

  function old new delta

  convert_to_struct - 351 +351

  parse_common - 214 +214

 ...

  bb__pgsreader 194 - -194

  bb__parsegrent 246 - -246

 
  --

  (add/remove: 16/10 grow/shrink: 4/6 up/down: 1411/-1293) Total: 118
  bytes





 I see the following bloatcheck:



 function old new delta

 convert_to_struct - 369 +369

 parse_common - 236 +236

 tokenize - 133 +133

 getgrouplist_internal 195 328 +133

 getpw_common - 97 +97

 getgr_common - 97 +97

 getpw_common_malloc - 81 +81

 getgr_common_malloc - 81 +81

 parse_file - 76 +76

 bb_internal_getpwent_r 102 148 +46

 bb_internal_getgrent_r 102 148 +46

 my_pwd - 28 +28

 my_grp - 16 +16

 sp_def - 4 +4

 pwd_buffer_size - 4 +4

 pwd_buffer - 4 +4

 pw_def - 4 +4

 my_pw - 4 +4

 my_gr - 4 +4

 grp_buffer_size - 4 +4

 grp_buffer - 4 +4

 gr_def - 4 +4

 gr_off 3 4 +1

 sp_off 9 8 -1

 ptr_to_statics 4 - -4

 bb_internal_getpwuid 38 19 -19

 bb_internal_getspnam_r 121 96 -25

 bb_internal_getgrgid 44 19 -25

 bb_internal_getpwnam 38 11 -27

 get_S 30 - -30

 bb_internal_getgrnam 44 11 -33

 bb_internal_fgetpwent_r 51 - -51

 bb_internal_fgetgrent_r 51 - -51

 bb_internal_getpwuid_r 113 48 -65

 bb_internal_getgrgid_r 113 48 -65

 bb_internal_getpwnam_r 121 40 -81

 bb_internal_getgrnam_r 121 40 -81

 bb__parsepwent 110 - -110

 bb__parsespent 120 - -120

 bb__pgsreader 213 - -213

 bb__parsegrent 226 - -226


 --

 (add/remove: 19/8 grow/shrink: 4/10 up/down: 1476/-1227) Total: 249 bytes

 text data bss dec hex filename

 923471 928 17684 942083 e6003 busybox_old

 923708 936 17744 942388 e6134 busybox_unstripped



 The growth of data and bss needs fixing. Old code contained

 infrastructure which folded all data into one on-demand allocated area.

 Let's use a similar trick here?



 tokenize() fails to strip trailing whitespace from last field.



 get_record_line() returns ERANGE if buffer is too small. Callers

 retry when they see that. Why can't get_record_line() itself reallocate

 the buffer, avoiding retry logic altogether?

 Sounds suspiciously like our good old xmalloc_fgetline() ;)



 + if (def[idx] == 'm') {

 + char *s 

Re: custom snmpset coding not working-giving-error

[Laszlo Papp]

Please stop sending these messages to this mailing list. This is _not_
an SNMP mailing list. If you need programming help, go to trainings or
other sites, like Stack Overflow.

On Mon, Dec 8, 2014 at 5:57 AM, Akhilesh kumar 95aku...@gmail.com wrote:
 Hi all

 i am writing my own implementation code for snmpset command. I have written
 the below code but it gives error. I cant figure out how to correct Bad
 variable type error.



 #include stdio.h
 #include net-snmp/net-snmp-config.h
 #include net-snmp/net-snmp-includes.h
 #include string.h
 #include net-snmp/varbind_api.h

 int main(int argc, char ** argv)
 {

 struct snmp_session session;
 struct snmp_session *sess_handle;
 struct snmp_pdu *pdu;
 struct snmp_pdu *response;
 struct variable_list *vars;
 oid id_oid[MAX_OID_LEN];
 size_t id_len = MAX_OID_LEN;
 size_t name_length;
 int status;
 char set_var_cval[30],set_var[50];
 int choose_option=0;
 char *p=NULL, get_var[51];
 int set_var_ival=0,failures=0,count;
 char   *names[SNMP_MAX_CMDLINE_OIDS];
 chartypes[SNMP_MAX_CMDLINE_OIDS];
 unsigned int*values[SNMP_MAX_CMDLINE_OIDS];
 oid name[MAX_OID_LEN];
 char type='i';
 const char
 *dirname=/usr/local/share/snmp/mibs/,*filename=DDS-ENGINE-MIB.mib;
 unsigned int value_ptr;



 add_mibdir(/usr/local/share/snmp/mibs/);
 read_mib(DDS-ENGINE-MIB.mib);
 printf(Enter the OID to set value\n);
 scanf(%s,set_var);
 printf(The OID to be set is %s\n,set_var);

 init_snmp(snmpdemoapp);
 snmp_sess_init( session );
 session.version = SNMP_VERSION_1;
 session.community = public;
 session.community_len = strlen(session.community);
 session.peername = argv[1];
 sess_handle = snmp_open(session);
 pdu = snmp_pdu_create(SNMP_MSG_SET);


 count=0;
 names[0]=set_var;
 printf(Enter the value to be set with\n);
 scanf(%d,value_ptr);
 values[0]=value_ptr;
 types[0]=i;
 name_length=strlen(names[0]);

 if (snmp_add_var(pdu, name, name_length, types[count],
 values[count]))
 { snmp_perror(names[count]); failures++; }



 status = snmp_synch_response(sess_handle, pdu, response);






 snmp_free_pdu(response);
 snmp_close(sess_handle);
 exit(1);

 }


 ___
 busybox mailing list
 busybox@busybox.net
 http://lists.busybox.net/mailman/listinfo/busybox
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Integrate snmp to busybox

[Laszlo Papp]

On Tue, Nov 4, 2014 at 9:13 AM, Akhilesh kumar 95aku...@gmail.com wrote:
 ok correct i got it.

 if i have my file system ,kernel and the busybox already on my board, and i
 want snmp support on my board but without reflashing
 anything(kernel/filesystem/bootloader). Is it possible. I thought i will put
 the source code files and the library files in the busybox/networking
 directory. But u say that busybox don't for c libraries.

 I also want the wput.c to be in my busybox like other networking commands.
 Is it possible.

Why would you need to reflash anything for putting a program on your
board on top of the Linux kernel? That would be very inconvenient,
wouldn't it? The technology has been a bit ahead for a couple of
decades now, Akhilesh.

Again, as Goetz wrote correct, busybox is not a container for
everything. If you want to put the wput executable (not wput.c, no, as
that is the source unless you want to build on the target), just put
it. Use Yocto, buildroot or some manual system that does it for you.

 On Tue, Nov 4, 2014 at 2:14 PM, Goetz Salzmann b...@blacknet.de wrote:


 Dear Akhilesh kumar,

   could you elaborate on why you want to do this, and what functionality
   you want to achieve.

  i have only busybox as an option to provide to my board
  ...
  i want to have snmp support on my board through busybox. So that i can
  monitor my board when i connect it on network.


 you apparently have no clue what you're talking about.  Please do some
 basic research.

 Some pointret:
 - busybox is NOT a Linux distribution.
 - busybox is a collection of basic (system) utilities to get a small
   Linux system running, but it does neither provide a kernel nor a c
   library or a bootloader.
 - you do NOT have only busybox as an option to privide to my board
   (I can only guess what you mean by that).
 - you could use the SDK for your board and add an SNMPd like mini-snmpd.
 - there are better options for monitoring small systems than snmp, like
   collectd.

 Cu,
 Goetz.

 ___
 busybox mailing list
 busybox@busybox.net
 http://lists.busybox.net/mailman/listinfo/busybox



 ___
 busybox mailing list
 busybox@busybox.net
 http://lists.busybox.net/mailman/listinfo/busybox
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Integrate snmp to busybox

[Laszlo Papp]

On Mon, Nov 3, 2014 at 6:39 PM,  b...@blacknet.de wrote:
 Hi Akhilesh,

 i want to integrate snmp(simple network management) protocol with busybox.
 Can anyone suggest me how to proceed or steps required.

 could you elaborate on why you want to do this, and what functionality
 you want to achieve.

Yeah, that was my initial thought when seeing this, but did not bother
to reply. Why is this necessary? I mean, do not get me wrong, we use
SNMP as well and all that, but I do not feel it so common that it
deserves to be in busybox. Nor do I see the problem with the normal
edition. It is a bit niche software and protocol in networking.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: `busyboxvi` is not working like before

[Laszlo Papp]

On Sun, Oct 19, 2014 at 3:05 PM, Laurent Bercot
ska-dietl...@skarnet.org wrote:
 Le 19/10/2014 15:22, walter harms a écrit :

 I would vote for a busybox-next (or what you call it).
 A branch that easly fold back into the main but contains all
 the recent patches.


  That may be desirable indeed.
  However, *forking* the project would be a terrible idea.

Why? If Denis does not have time for the project, then we cannot
really blame him. On the other hand, if a fork could work better with
someone standing up, why not?

Hopefully this is not just yet another omgz, you are forking, what a
terrible person political argument. I prefer pragmatism to politics.

Cheers, L.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: `busyboxvi` is not working like before

[Laszlo Papp]

On Fri, Oct 31, 2014 at 6:11 PM, Laurent Bercot
ska-dietl...@skarnet.org wrote:
 Why? If Denis does not have time for the project, then we cannot
 really blame him. On the other hand, if a fork could work better with
 someone standing up, why not?


  Forking a project divides resources and weakens both children. It is
 a possibility for big projects with lots of resources and several leaders
 with conflicting goals.

That seems to be wrong in my view. I am sorry if that sounds snarky.
Check out the libressl project which seems to be going into the right
direction. You cannot really say that the openssl project was
overloaded with contributors. Yet, my several years old vulnerability
fixes were struggling to get in. In fact, I did not even get any
reviews for them. This is not an issue with the libressl fork anymore.

 But when the problem is finding a maintainer with
 enough time, expertise and willingness to maintain a project, forking is
 definitely not a solution - on the contrary, it would only exacerbate the
 problem. You were struggling to find one person, now you must find two.

Not quite, no. What I see is that Denis is the ultimate maintainer and
*no one* else is allowed to commit changes from the contributors.

On the contrary, I have seen a *lot* of bikeshed on this mailing list
about insignificant details. The end results usually were missing
features due to completely irrelevant implementation details in my
opinion. I still remember one of my first patches that got more than
100 emails in that thread and the whole thing was about bikeshedding,
again IMHO. Apparently, many people have resource for that, but not
actual pragmaticism!

  Someone standing up is not enough. What will work is someone with
 time, expertise and willingness to maintain a project such as Busybox
 standing up.

Do you really think anyone of us think it otherwise? Assuming
maturity, I cannot possibly imagine why you would think it otherwise.

 So far I haven't seen anyone checking all three boxes better
 than Denys, so don't be so quick to replace him, pretty please.

That shows why I think you do not get what I am trying to write. My
problem is not with Denys' skills. My problem is with Denys' time
_and_ ultimate role without others being able to maintain certain
parts.

busybox --list | wc -l
351

Does anyone here really think that one person can maintain 351 applets
alone? I think it should be acceptable for others to stand up and not
to have an exclusive person here responsible for everything. I am sure
that there have been applet authors who would have been happy to give
a helping hand with maintaining their code pieces.

Cheers, L.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [RFC] malloced getpw/grxxx functions for bb

[Laszlo Papp]

Sad to see your monologue. :-(

For my money, it is not that bad, so it would be nice to get a
feedback from the ultimate maintainer. As far as I can see it is a
very serious hazard in busybox that you are trying to address
following my initial email.

On Sun, Sep 28, 2014 at 2:24 PM, tito farmat...@tiscali.it wrote:
 On Saturday 27 September 2014 14:58:08 tito wrote:
 On Wednesday 24 September 2014 23:02:55 tito wrote:
  On Saturday 20 September 2014 16:32:01 tito wrote:
   Hi,
   One more fix of a return value.
  
   Ciao,
   Tito
  
 
  Hi,
  more return value and errno fixes.
 
  Ciao,
  Tito
 
 Hi,
 make the tokenize function more robust.

 Ciao,
 Tito

 Hi,
 more minor errno fixes and code cleanups.

 Ciao,
 Tito

 ___
 busybox mailing list
 busybox@busybox.net
 http://lists.busybox.net/mailman/listinfo/busybox
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Fix the addgroup help output

[Laszlo Papp]

Isaac,

I honestly took my time to address your concerns, but I have never
received any feedback. Please ... ?

Cheers, L.

On Thu, Aug 21, 2014 at 2:43 PM, Laszlo Papp lp...@kde.org wrote:
 On Thu, Aug 21, 2014 at 2:39 PM, Laszlo Papp lp...@kde.org wrote:
 On Mon, Aug 18, 2014 at 4:39 PM, Isaac Dunham ibid...@gmail.com wrote:
 On Mon, Aug 18, 2014 at 01:51:00PM +0100, Laszlo Papp wrote:
 You completely miss the point. The whole point of the thread is not about
 writing down one personal preference and done. The applets will be still
 inconsistent, and the process will require manual reviews, style
 investigation, and what not. Any reasonable programmer knows that this can
 be automated without serious defect. Others project have done it; I am sure
 busybox would be able to do it if wanted, but I realize this list has heavy
 objection for almost any valid improvement which kind of makes me burning
 out providing suggestions and comments, but that is not a discussion for
 today...

 So far I've seen This needs to be done *differently* without much of an
 explanation of what differently looks like, and doing it differently
 won't work with little understanding of what the alternatives are.
 (And no one trying to look at all the alternatives with pros and cons.)

 The alternatives I can think of:
 * have a usage() function for each command.
  This is the way most/many command-line programs are written.
  It sucks worse than what we do.

 * the old way. Also does not work.

 * keep doing it the current way.
  I'm annoyed by writing the //usage: \n part; Laszlo is complaining
  that this approach leaves it manually maintained.

 * move //usage: to just above where each option is implemented.
  Easier to check consistency with code; harder to see functionality.

 * toybox style.
 This puts it in the comment at the top:
 /* command - explanation
  * copyright 2014 Foo Bar
  * blah blah blah
 USE_COMMAND(...) //wraps macro to add new target/help text/option 
 parsing/...
 config COMMAND
   bool command
   default y
   help
 usage: command [-ab] [-f FOO]

 Does something to file FOO
 -a Do something
 -b Do something else
 -f Read input from FOO
 */

  This can be ignored and get out of sync with the code, can be brittle,
  but provides one informative help text for both kconfig and the toy/applet.
  It also doesn't require writing //usage:  \n for every line, and then
  doing //kconfig:

 * have our getopt alternative use an array like getopt_long() does.
  This mandates changing everything, using getopt32_new for anything we want
  to document, cannot be compressed, means a bit of bloat, and may have other
  limitations.


 Is there another way that I'm not thinking of, or would anyone like to
 expand on the limitations of one of these?
 When you refer to another way, please give a sample of what it would
 look like, and try to describe what the limits are.

 Yes, there is another (and better) way IMHO. Let us put the help text
 of the applet here that the original patch in this thread was
 touching:

 //usage:#define addgroup_trivial_usage
 //usage:   [-g GID] [-S]  IF_FEATURE_ADDUSER_TO_GROUP([USER] ) 
 GROUP
 //usage:#define addgroup_full_usage \n\n
 //usage:   Add a group IF_FEATURE_ADDUSER_TO_GROUP( or add a
 user to a group) \n
 //usage: \n-g GID  Group id
 //usage: \n-S  Create a system group

 Proposal:

 /* @usage: desc
  * -g/--gid argdesc
  * -S
  */

 It is just for the very basic, but I am sure the syntax could be
 extended to allow custom cookies and brownies, too. Then, the
 buildsystem would generate the code for the actual help text in the
 background.

 This is obviously C comment hack, but this could also be made into a
 nicer descriptive language, like json, etc. That is not a big problem
 AFAICT, but the idea remains, only type the metadata, not the all the
 noise.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH v2] Ntpd config file support

[Laszlo Papp]

How about ... for instance merging this thing ... ?

On Mon, Mar 24, 2014 at 3:13 AM, Isaac Dunham ibid...@gmail.com wrote:
 On Sun, Mar 23, 2014 at 03:06:08PM +0100, Denys Vlasenko wrote:
 On Saturday 22 March 2014 23:46, Isaac Dunham wrote:
  On Sat, Mar 22, 2014 at 08:40:48PM +0100, Harald Becker wrote:
   Hi Isaac !
  
   Your program will fail on lines starting with the word server
   (eg. serverxyz), that is it does not check for clear word
   boundary and gives wrong results in that case.
 
  ...which are not legitimate entries in ntp.conf.
 
  My aim is to parse a correct ntp.conf, and not cause security problems
  on incorrect ones.

 bbox has config parsing routines to avoid coding this again and again.

 How about this?

 function old new   delta
 add_peers  -  98 +98
 packed_usage   29470   29511 +41
 ntp_init 407 428 +21
 pw_encrypt14  27 +13
 --
 (add/remove: 1/0 grow/shrink: 3/0 up/down: 173/0) Total: 173 
 bytes


 Definitely better than mine.

 I'm inclined to agree with Harald as to the skipping messages;
 _if_ they are included, they should happen in verbose mode only.

 Any idea why pw_encrypt is changing?

 Now, regarding the default n vs default y/whether compat should
 always be/is always on...
 I'm aware of at least two compatability options that default to the
 smaller less compatible version:
 CONFIG_MODPROBE_SMALL is certainly not more compatible (-ablD being
 missing in the small version.)
 CONFIG_EXTRA_COMPAT is default n, and that is a good thing.

 All the people I've see asking for the feature use distro binaries,
 rather than defconfig.

 That said, I take no position on whether to make this default n.

 Thanks,
 Isaac Dunham

 ___
 busybox mailing list
 busybox@busybox.net
 http://lists.busybox.net/mailman/listinfo/busybox
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [RFC] malloced getpw/grxxx functions for bb

[Laszlo Papp]

On Fri, Oct 31, 2014 at 8:13 PM, tito farmat...@tiscali.it wrote:
 On Friday 31 October 2014 19:40:56 you wrote:
 Sad to see your monologue. :-(

 For my money, it is not that bad, so it would be nice to get a
 feedback from the ultimate maintainer. As far as I can see it is a
 very serious hazard in busybox that you are trying to address
 following my initial email.

 Hi,
 it was a lot of fun and a lot of stuff to learn.
 I think it could be still improved if somebody
 of the professional developers (not self-taught like me)
 could take a look at it.
 I doubt that this code will ever make it into bb
 as it would be a hazard to replace well proven
 code with this rewrite, nonetheless I did my best.

The well proven code is quite broken for said reasons. Currently, it
is possible to very easily blow a system up with it. If fixing such a
serious hazard cannot make it in due to some policies, that sounds
like an indication for thinking about a fork in my opinion. It is
unreasonable to leave such serious vulnerabilities in a system.

Cheers, L.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: adduser/passwd: too long username

[Laszlo Papp]

On Thu, Aug 21, 2014 at 7:37 AM, Natanael Copa nc...@alpinelinux.org wrote:
 On Mon, 18 Aug 2014 20:37:00 +0200
 Natanael Copa nc...@alpinelinux.org wrote:

 I never sent those patches upstream because I don't think this is the
 correct fix. For Alpine Linux the correct fix was to use the libc
 implementation and a libc that handles this properly (musl libc)
 together with the patch I sent to this mailing list.

 http://lists.busybox.net/pipermail/busybox/2014-April/080809.html

 Commit message didn't say it but that patch is also needed to fix the
 username/groupsize issue - with glibc too I believe.

 Denys, any chance that the above mentioned patch gets applied?

 Should I send a v2 with better commit message?

No, please check out tito's work. That seems to be the right to do.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Fix the addgroup help output

[Laszlo Papp]

On Mon, Aug 18, 2014 at 4:39 PM, Isaac Dunham ibid...@gmail.com wrote:
 On Mon, Aug 18, 2014 at 01:51:00PM +0100, Laszlo Papp wrote:
 You completely miss the point. The whole point of the thread is not about
 writing down one personal preference and done. The applets will be still
 inconsistent, and the process will require manual reviews, style
 investigation, and what not. Any reasonable programmer knows that this can
 be automated without serious defect. Others project have done it; I am sure
 busybox would be able to do it if wanted, but I realize this list has heavy
 objection for almost any valid improvement which kind of makes me burning
 out providing suggestions and comments, but that is not a discussion for
 today...

 So far I've seen This needs to be done *differently* without much of an
 explanation of what differently looks like, and doing it differently
 won't work with little understanding of what the alternatives are.
 (And no one trying to look at all the alternatives with pros and cons.)

 The alternatives I can think of:
 * have a usage() function for each command.
  This is the way most/many command-line programs are written.
  It sucks worse than what we do.

 * the old way. Also does not work.

 * keep doing it the current way.
  I'm annoyed by writing the //usage: \n part; Laszlo is complaining
  that this approach leaves it manually maintained.

 * move //usage: to just above where each option is implemented.
  Easier to check consistency with code; harder to see functionality.

 * toybox style.
 This puts it in the comment at the top:
 /* command - explanation
  * copyright 2014 Foo Bar
  * blah blah blah
 USE_COMMAND(...) //wraps macro to add new target/help text/option parsing/...
 config COMMAND
   bool command
   default y
   help
 usage: command [-ab] [-f FOO]

 Does something to file FOO
 -a Do something
 -b Do something else
 -f Read input from FOO
 */

  This can be ignored and get out of sync with the code, can be brittle,
  but provides one informative help text for both kconfig and the toy/applet.
  It also doesn't require writing //usage:  \n for every line, and then
  doing //kconfig:

 * have our getopt alternative use an array like getopt_long() does.
  This mandates changing everything, using getopt32_new for anything we want
  to document, cannot be compressed, means a bit of bloat, and may have other
  limitations.


 Is there another way that I'm not thinking of, or would anyone like to
 expand on the limitations of one of these?
 When you refer to another way, please give a sample of what it would
 look like, and try to describe what the limits are.

Yes, there is another (and better) way IMHO. Let us put the help text
of the applet here that the original patch in this thread was
touching:

//usage:#define addgroup_trivial_usage
//usage:   [-g GID] [-S]  IF_FEATURE_ADDUSER_TO_GROUP([USER] ) GROUP
//usage:#define addgroup_full_usage \n\n
//usage:   Add a group IF_FEATURE_ADDUSER_TO_GROUP( or add a
user to a group) \n
//usage: \n-g GID  Group id
//usage: \n-S  Create a system group

Proposal:

/* @usage: desc
 * -g/--gid argdesc
 * -S
 */

It is just for the very basic, but I am sure the syntax could be
extended to allow custom cookies and brownies, too. Then, the
buildsystem would generate the code for the actual help text in the
background.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Fix the addgroup help output

[Laszlo Papp]

On Thu, Aug 21, 2014 at 2:39 PM, Laszlo Papp lp...@kde.org wrote:
 On Mon, Aug 18, 2014 at 4:39 PM, Isaac Dunham ibid...@gmail.com wrote:
 On Mon, Aug 18, 2014 at 01:51:00PM +0100, Laszlo Papp wrote:
 You completely miss the point. The whole point of the thread is not about
 writing down one personal preference and done. The applets will be still
 inconsistent, and the process will require manual reviews, style
 investigation, and what not. Any reasonable programmer knows that this can
 be automated without serious defect. Others project have done it; I am sure
 busybox would be able to do it if wanted, but I realize this list has heavy
 objection for almost any valid improvement which kind of makes me burning
 out providing suggestions and comments, but that is not a discussion for
 today...

 So far I've seen This needs to be done *differently* without much of an
 explanation of what differently looks like, and doing it differently
 won't work with little understanding of what the alternatives are.
 (And no one trying to look at all the alternatives with pros and cons.)

 The alternatives I can think of:
 * have a usage() function for each command.
  This is the way most/many command-line programs are written.
  It sucks worse than what we do.

 * the old way. Also does not work.

 * keep doing it the current way.
  I'm annoyed by writing the //usage: \n part; Laszlo is complaining
  that this approach leaves it manually maintained.

 * move //usage: to just above where each option is implemented.
  Easier to check consistency with code; harder to see functionality.

 * toybox style.
 This puts it in the comment at the top:
 /* command - explanation
  * copyright 2014 Foo Bar
  * blah blah blah
 USE_COMMAND(...) //wraps macro to add new target/help text/option parsing/...
 config COMMAND
   bool command
   default y
   help
 usage: command [-ab] [-f FOO]

 Does something to file FOO
 -a Do something
 -b Do something else
 -f Read input from FOO
 */

  This can be ignored and get out of sync with the code, can be brittle,
  but provides one informative help text for both kconfig and the toy/applet.
  It also doesn't require writing //usage:  \n for every line, and then
  doing //kconfig:

 * have our getopt alternative use an array like getopt_long() does.
  This mandates changing everything, using getopt32_new for anything we want
  to document, cannot be compressed, means a bit of bloat, and may have other
  limitations.


 Is there another way that I'm not thinking of, or would anyone like to
 expand on the limitations of one of these?
 When you refer to another way, please give a sample of what it would
 look like, and try to describe what the limits are.

 Yes, there is another (and better) way IMHO. Let us put the help text
 of the applet here that the original patch in this thread was
 touching:

 //usage:#define addgroup_trivial_usage
 //usage:   [-g GID] [-S]  IF_FEATURE_ADDUSER_TO_GROUP([USER] ) GROUP
 //usage:#define addgroup_full_usage \n\n
 //usage:   Add a group IF_FEATURE_ADDUSER_TO_GROUP( or add a
 user to a group) \n
 //usage: \n-g GID  Group id
 //usage: \n-S  Create a system group

 Proposal:

 /* @usage: desc
  * -g/--gid argdesc
  * -S
  */

 It is just for the very basic, but I am sure the syntax could be
 extended to allow custom cookies and brownies, too. Then, the
 buildsystem would generate the code for the actual help text in the
 background.

This is obviously C comment hack, but this could also be made into a
nicer descriptive language, like json, etc. That is not a big problem
AFAICT, but the idea remains, only type the metadata, not the all the
noise.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: adduser/passwd: too long username

[Laszlo Papp]

On Thu, Aug 21, 2014 at 10:10 PM, Natanael Copa n...@tanael.org wrote:
 Den 21. aug. 2014 kl. 08:42 skrev Laszlo Papp lp...@kde.org:

 On Thu, Aug 21, 2014 at 7:37 AM, Natanael Copa nc...@alpinelinux.org wrote:
 On Mon, 18 Aug 2014 20:37:00 +0200
 Natanael Copa nc...@alpinelinux.org wrote:

 I never sent those patches upstream because I don't think this is the
 correct fix. For Alpine Linux the correct fix was to use the libc
 implementation and a libc that handles this properly (musl libc)
 together with the patch I sent to this mailing list.

 http://lists.busybox.net/pipermail/busybox/2014-April/080809.html

 Commit message didn't say it but that patch is also needed to fix the
 username/groupsize issue - with glibc too I believe.

 Denys, any chance that the above mentioned patch gets applied?

 Should I send a v2 with better commit message?

 No, please check out tito's work. That seems to be the right to do.

 we need both.

 as i understand, titos patch solves the problem for when you enable busybox's 
 implementation with CONFIG_USE_BB_PWD_GRP. you actually have the same problem 
 with it disabled, even if system implementation can handle it correctly. that 
 is what my patch solves.

That seem to be an irrelevant discussion in this thread, then. Should
have been sent to a separate thread. This thread is about issues when
CONFIG_USE_BB_PWD_GRP is enabled.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [RFC] malloced getpw/grxxx/_r functions for bb

[Laszlo Papp]

How do you prove that your rewrite from scratch is robust?

On Mon, Aug 18, 2014 at 8:50 PM, tito farmat...@tiscali.it wrote:
 On Tuesday 05 August 2014 20:16:04 Denys Vlasenko wrote:
 On Mon, Aug 4, 2014 at 7:06 PM, Laszlo Papp lp...@kde.org wrote:
  sudo busybox adduser
  f
  passwd: unknown user
  f
 
  Yet, the user is created in /etc/shadow:
 
  f:!:16286:0:9:7:::
 
  This is at least one issue, but there is another here:
 
  sudo busybox deluser
  f
  deluser: unknown user
  f

 Both issues come from the same location in codebase:
 bb__pgsreader() parser drops lines which are longer than its buffer.

 Effectively, bbox ignores offending line in /etc/passwd.

  Could you please look into this and potentially fix it? Thanks in advance.

 Anyone willing to rewrite getpwnam API to use variable-sized malloced buffer?

 Hi,
 i couldn 't resist as I was sure there was a lot to learn and so I've started
 to reinvent the wheel. At first I've tried to modify the original bb 
 implementation
 but the code with its preprocessor black magic made it difficult so I've 
 started
 from scratch.
 At the moment I have a proof of concept implementation for:

 getpwuid
 getgrgid
 getpwnam
 getgrnam
 getpwuid_r
 getgrgid_r
 getpwnam_r

 The code is ugly and probably bloated for bb so look at it at your own risk 
 :-)

 Other functions still missing but used in bb:
 fgetpwent_r
 fgetgrent_r
 getspnam_r
 setpwent
 endpwent
 setgrent
 endgrent
 getpwent_r
 getgrent_r
 initgroups

 What it does:
 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 _NEEDED_ size and reused for later calls.

 2) the fields in the passwd files are checked and if empty (in some cases) or 
 containing
 whitespace, etc. a error message about malformed line in file xx
 is printed so that the user knows instead of being silently ignored.

 3) the _r functions use the user supplied buffers that are never reallocated
 but use mostly the same common functions.

 4) there was something

 Before going further some review and improvements (size reduction, obvious 
 errors, bugs
 and other horrors) by the list members is needed  and also some hints if this 
 implementation
 makes sense at all or if I should forget about it.

 My biggest problem at the moment is a memory leak in handling the members in
 group-gr_mem and I don't see a obvious way to fix it.

 The proof of concept is developed out of tree in the pwdgrp.c file that 
 contains also
 a test program to grossly check if everything works. The relevant libbb 
 functions are
 copied over to the libbb.c file which is included automatically. You can 
 compile it simply by:

 gcc pwdgrp.c -o test -Wall


 valgrind of a test run shows:

 =7229== HEAP SUMMARY:
 ==7229== in use at exit: 272 bytes in 4 blocks
 ==7229==   total heap usage: 22 allocs, 18 frees, 4,012 bytes allocated
 ==7229==
 ==7229== 68 bytes in 1 blocks are still reachable in loss record 1 of 4
 ==7229==at 0x4028308: malloc (vg_replace_malloc.c:263)
 ==7229==by 0x402849F: realloc (vg_replace_malloc.c:632)
 ==7229==by 0x80489E4: xrealloc (in /home/tito/Desktop/test)
 ==7229==by 0x8048A5C: xrealloc_vector_helper (in /home/tito/Desktop/test)
 ==7229==by 0x8049584: convert_to_grp (in /home/tito/Desktop/test)
 ==7229==by 0x80498AB: my_getgr_common (in /home/tito/Desktop/test)
 ==7229==by 0x80498FE: my_getgrgid (in /home/tito/Desktop/test)
 ==7229==by 0x8049EBD: main (in /home/tito/Desktop/test)
 ==7229==
 ==7229== 68 bytes in 1 blocks are definitely lost in loss record 2 of 4
 ==7229==at 0x4028308: malloc (vg_replace_malloc.c:263)
 ==7229==by 0x402849F: realloc (vg_replace_malloc.c:632)
 ==7229==by 0x80489E4: xrealloc (in /home/tito/Desktop/test)
 ==7229==by 0x8048A5C: xrealloc_vector_helper (in /home/tito/Desktop/test)
 ==7229==by 0x804953E: convert_to_grp (in /home/tito/Desktop/test)
 ==7229==by 0x80498AB: my_getgr_common (in /home/tito/Desktop/test)
 ==7229==by 0x80498CD: my_getgrnam (in /home/tito/Desktop/test)
 ==7229==by 0x8049D43: main (in /home/tito/Desktop/test)
 ==7229==
 ==7229== 68 bytes in 1 blocks are definitely lost in loss record 3 of 4
 ==7229==at 0x4028308: malloc (vg_replace_malloc.c:263)
 ==7229==by 0x402849F: realloc

Re: [RFC] malloced getpw/grxxx/_r functions for bb

[Laszlo Papp]

On Wed, Aug 20, 2014 at 8:12 AM, tito farmat...@tiscali.it wrote:
 On Wednesday 20 August 2014 08:12:08 you wrote:
 How do you prove that your rewrite from scratch is robust?
 Hi,
 I've done minimal testing through the included test program
 and I've done a few valgrind tet runs on every revision until now.
 Still I cannot prove any robustness. That is why I've posted
 the idea and a proof of concept implementation that needs
 more work and cleanup to list where more experienced programmers
 can if they so want review that code and suggest fixes, improvements
 and cleanups or just tell me: Forget about it.

Well, it is not the last step to make it robust. It is an upfront
decision and development paradigm. If you cannot make it robust, it
may cause more harm than what you are trying to resolve. This code has
been in busybox for a while, I assume. You need to make sure that what
you intend to solve does work. I would suggest TDD here. Write test
cases for all the cases that is missing and initially they will fail.
Once you get them all passing, it is time to consider. IMHO, this
should be done before the feature development and internal software
design decision since it well defines what you are trying to achieve.

 Ciao,
 Tito


  That's why I post it to the
 On Mon, Aug 18, 2014 at 8:50 PM, tito farmat...@tiscali.it wrote:
  On Tuesday 05 August 2014 20:16:04 Denys Vlasenko wrote:
  On Mon, Aug 4, 2014 at 7:06 PM, Laszlo Papp lp...@kde.org wrote:
   sudo busybox adduser
   f
   passwd: unknown user
   f
  
   Yet, the user is created in /etc/shadow:
  
   f:!:16286:0:9:7:::
  
   This is at least one issue, but there is another here:
  
   sudo busybox deluser
   f
   deluser: unknown user
   f
 
  Both issues come from the same location in codebase:
  bb__pgsreader() parser drops lines which are longer than its buffer.
 
  Effectively, bbox ignores offending line in /etc/passwd.
 
   Could you please look into this and potentially fix it? Thanks in 
   advance.
 
  Anyone willing to rewrite getpwnam API to use variable-sized malloced 
  buffer?
 
  Hi,
  i couldn 't resist as I was sure there was a lot to learn and so I've 
  started
  to reinvent the wheel. At first I've tried to modify the original bb 
  implementation
  but the code with its preprocessor black magic made it difficult so I've 
  started
  from scratch.
  At the moment I have a proof of concept implementation for:
 
  getpwuid
  getgrgid
  getpwnam
  getgrnam
  getpwuid_r
  getgrgid_r
  getpwnam_r
 
  The code is ugly and probably bloated for bb so look at it at your own 
  risk :-)
 
  Other functions still missing but used in bb:
  fgetpwent_r
  fgetgrent_r
  getspnam_r
  setpwent
  endpwent
  setgrent
  endgrent
  getpwent_r
  getgrent_r
  initgroups
 
  What it does:
  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 _NEEDED_ size and reused for later calls.
 
  2) the fields in the passwd files are checked and if empty (in some cases) 
  or containing
  whitespace, etc. a error message about malformed line in file xx
  is printed so that the user knows instead of being silently ignored.
 
  3) the _r functions use the user supplied buffers that are never 
  reallocated
  but use mostly the same common functions.
 
  4) there was something
 
  Before going further some review and improvements (size reduction, obvious 
  errors, bugs
  and other horrors) by the list members is needed  and also some hints if 
  this implementation
  makes sense at all or if I should forget about it.
 
  My biggest problem at the moment is a memory leak in handling the members 
  in
  group-gr_mem and I don't see a obvious way to fix it.
 
  The proof of concept is developed out of tree in the pwdgrp.c file that 
  contains also
  a test program to grossly check if everything works. The relevant libbb 
  functions are
  copied over to the libbb.c file which is included automatically. You can 
  compile it simply by:
 
  gcc pwdgrp.c -o test -Wall
 
 
  valgrind of a test run shows:
 
  =7229== HEAP SUMMARY:
  ==7229== in use at exit: 272 bytes in 4 blocks
  ==7229==   total heap usage: 22 allocs, 18 frees, 4,012 bytes allocated
  ==7229==
  ==7229== 68 bytes in 1 blocks are still reachable in loss record 1 of 4

Re: [PATCH 1/3] testsuite: warning when bash is not in use.

[Laszlo Papp]

On Tue, Aug 19, 2014 at 7:24 AM, Natanael Copa nc...@alpinelinux.org
wrote:

 On Mon, 18 Aug 2014 00:11:10 -0300
 Guilherme Maciel Ferreira guilherme.maciel.ferre...@gmail.com wrote:

  The test suite scripts fail with Bad substitution error when using a
 shell
  other than Bash. This patch adds an alert to the user, to remind him to
 change
  its /bin/sh symlink.

 Wouldn't it be better to do:


Or even eliminate the bashism?


 diff --git a/testsuite/runtest b/testsuite/runtest
 index 51575d9..35968b1 100755
 --- a/testsuite/runtest
 +++ b/testsuite/runtest
 @@ -1,4 +1,4 @@
 -#!/bin/sh
 +#!/bin/bash
  # Usage:
  # runtest [applet1] [applet2...]


 Or even better, fix the script to be posix compliant?


Yep, exactly... +1 to this.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Fix the addgroup help output

[Laszlo Papp]

On Mon, Aug 18, 2014 at 9:45 AM, walter harms wha...@bfs.de wrote:



 Am 17.08.2014 11:22, schrieb Denys Vlasenko:
  On Fri, Aug 15, 2014 at 3:11 PM, lp...@archlinux.us wrote:
 
   Applets are defining the help display text on their own, and it is
  different for different applets.
 
 
  I don't see any obvious way to make it easier without significant bloat.
 
  Pass a list of options, parameters and their explanations to a function
  which builds help text? This will likely be bigger than the help text
  (many pointers to small strings results in pointers taking
  almost the same amount of space than strings!)
 
  Even if you manage to avoid _that_ bloat somehow,
  you can't bzip2-compress tiny bits of text.
  We currently bzip2-compress our help text as one big blob,
  with BIG savings in size - more than 50%
 
  And then you discover that some applets really want a bit
  of customization to the way how options are explained.
 
 

 Would that something for busybox.net/TODO section ?
 Verify that help text and option are in sync


Would what for TODO? It should not never really be validated by a human in
the first place. An applet author should just fill in the metadata. All the
rest should be automatically handled IMHO. Otherwise, it is error-prone,
will likely only cause the chaos that the applets have now with regards to
this.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Fix the addgroup help output

[Laszlo Papp]

On Mon, Aug 18, 2014 at 1:07 PM, tito farmat...@tiscali.it wrote:

 On Monday 18 August 2014 10:50:18 Laszlo Papp wrote:
  On Mon, Aug 18, 2014 at 9:45 AM, walter harms wha...@bfs.de wrote:
 
  
  
   Am 17.08.2014 11:22, schrieb Denys Vlasenko:
On Fri, Aug 15, 2014 at 3:11 PM, lp...@archlinux.us wrote:
   
 Applets are defining the help display text on their own, and it is
different for different applets.
   
   
I don't see any obvious way to make it easier without significant
 bloat.
   
Pass a list of options, parameters and their explanations to a
 function
which builds help text? This will likely be bigger than the help text
(many pointers to small strings results in pointers taking
almost the same amount of space than strings!)
   
Even if you manage to avoid _that_ bloat somehow,
you can't bzip2-compress tiny bits of text.
We currently bzip2-compress our help text as one big blob,
with BIG savings in size - more than 50%
   
And then you discover that some applets really want a bit
of customization to the way how options are explained.
   
   
  
   Would that something for busybox.net/TODO section ?
   Verify that help text and option are in sync
  
 
  Would what for TODO? It should not never really be validated by a human
 in
  the first place. An applet author should just fill in the metadata. All
 the
  rest should be automatically handled IMHO. Otherwise, it is error-prone,
  will likely only cause the chaos that the applets have now with regards
 to
  this.
 

 HI,
 if you would like to take a look at busybox/docs/BusyBox.html
 there are displayed all help texts, most of them look ok to me,
 a few could need some fixes, so lets list them. It would be
 also useful to write down exactly to what standards we want
 to enforce. KISS.


I am not sure what you are looking at. They are  completely inconsistent.
They use at least three different schema for just the options, let alone
others:

* [-fb]
* [-f] [b]
* [OPTIONS]

If that looks OK to you, then I guess you do not like unified and
simplified outputs. That is definitely not any KISS you are referring to. I
would advise you to have a closer look than what you have apparently done.

But let us imagine that for a second that what you are saying is true: it
is still extremely cumbersome, error-prone, and nonsensical IMHO to write
the help output yourself, either a rewrite for an existing or creating it
for a new one. The applet author ought to be responsible only for the
metadata, and no any more boilerplate.

I think you just simply do not see the forest from the trees here, or the
communication has just severely failed to bring the attention to the actual
issue.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Fix the addgroup help output

[Laszlo Papp]

On Mon, Aug 18, 2014 at 1:11 PM, Laszlo Papp lp...@kde.org wrote:




 On Mon, Aug 18, 2014 at 1:07 PM, tito farmat...@tiscali.it wrote:

 On Monday 18 August 2014 10:50:18 Laszlo Papp wrote:
  On Mon, Aug 18, 2014 at 9:45 AM, walter harms wha...@bfs.de wrote:
 
  
  
   Am 17.08.2014 11:22, schrieb Denys Vlasenko:
On Fri, Aug 15, 2014 at 3:11 PM, lp...@archlinux.us wrote:
   
 Applets are defining the help display text on their own, and it is
different for different applets.
   
   
I don't see any obvious way to make it easier without significant
 bloat.
   
Pass a list of options, parameters and their explanations to a
 function
which builds help text? This will likely be bigger than the help
 text
(many pointers to small strings results in pointers taking
almost the same amount of space than strings!)
   
Even if you manage to avoid _that_ bloat somehow,
you can't bzip2-compress tiny bits of text.
We currently bzip2-compress our help text as one big blob,
with BIG savings in size - more than 50%
   
And then you discover that some applets really want a bit
of customization to the way how options are explained.
   
   
  
   Would that something for busybox.net/TODO section ?
   Verify that help text and option are in sync
  
 
  Would what for TODO? It should not never really be validated by a human
 in
  the first place. An applet author should just fill in the metadata. All
 the
  rest should be automatically handled IMHO. Otherwise, it is error-prone,
  will likely only cause the chaos that the applets have now with regards
 to
  this.
 

 HI,
 if you would like to take a look at busybox/docs/BusyBox.html
 there are displayed all help texts, most of them look ok to me,
 a few could need some fixes, so lets list them. It would be
 also useful to write down exactly to what standards we want
 to enforce. KISS.


 I am not sure what you are looking at. They are  completely inconsistent.
 They use at least three different schema for just the options, let alone
 others:

 * [-fb]
 * [-f] [b]
 * [OPTIONS]

 If that looks OK to you, then I guess you do not like unified and
 simplified outputs. That is definitely not any KISS you are referring to. I
 would advise you to have a closer look than what you have apparently done.

 But let us imagine that for a second that what you are saying is true: it
 is still extremely cumbersome, error-prone, and nonsensical IMHO to write
 the help output yourself, either a rewrite for an existing or creating it
 for a new one. The applet author ought to be responsible only for the
 metadata, and no any more boilerplate.

 I think you just simply do not see the forest from the trees here, or the
 communication has just severely failed to bring the attention to the actual
 issue.


This is also what I mentioned previously that I do not think it makes sense
to apply a change for each individual case separately. I think it should be
addressed centrally rather than cluttering the log. By the way, the current
status quo reminds me these:

http://i698.photobucket.com/albums/vv350/FudgeSociety/copypasta.jpg

and

http://en.wikipedia.org/wiki/Copy_and_paste_programming
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Fix the addgroup help output

[Laszlo Papp]

On Mon, Aug 18, 2014 at 1:42 PM, tito farmat...@tiscali.it wrote:

 On Monday 18 August 2014 14:11:48 you wrote:
  On Mon, Aug 18, 2014 at 1:07 PM, tito farmat...@tiscali.it wrote:
 
   On Monday 18 August 2014 10:50:18 Laszlo Papp wrote:
On Mon, Aug 18, 2014 at 9:45 AM, walter harms wha...@bfs.de wrote:
   


 Am 17.08.2014 11:22, schrieb Denys Vlasenko:
  On Fri, Aug 15, 2014 at 3:11 PM, lp...@archlinux.us wrote:
 
   Applets are defining the help display text on their own, and
 it is
  different for different applets.
 
 
  I don't see any obvious way to make it easier without significant
   bloat.
 
  Pass a list of options, parameters and their explanations to a
   function
  which builds help text? This will likely be bigger than the help
 text
  (many pointers to small strings results in pointers taking
  almost the same amount of space than strings!)
 
  Even if you manage to avoid _that_ bloat somehow,
  you can't bzip2-compress tiny bits of text.
  We currently bzip2-compress our help text as one big blob,
  with BIG savings in size - more than 50%
 
  And then you discover that some applets really want a bit
  of customization to the way how options are explained.
 
 

 Would that something for busybox.net/TODO section ?
 Verify that help text and option are in sync

   
Would what for TODO? It should not never really be validated by a
 human
   in
the first place. An applet author should just fill in the metadata.
 All
   the
rest should be automatically handled IMHO. Otherwise, it is
 error-prone,
will likely only cause the chaos that the applets have now with
 regards
   to
this.
   
  
   HI,
   if you would like to take a look at busybox/docs/BusyBox.html
   there are displayed all help texts, most of them look ok to me,
   a few could need some fixes, so lets list them. It would be
   also useful to write down exactly to what standards we want
   to enforce. KISS.
  
 
  I am not sure what you are looking at. They are  completely inconsistent.
  They use at least three different schema for just the options, let alone
  others:

 Which one do you prefer?


It really does *not* matter what I prefer. The maintainer decides and it is
_purely_ subjective, so there is not much point in telling what I prefer.
That is the least significant point for me. Consistency is by far way above
any personal style in my book.



 This could be ok for indipendent options.
  * [-fb]
 This could be ok for options that depend on each other
 or eventually [-f[b]]
  * [-f] [b]
 THis could be ok for the short usage form  FEATURE_VERBOSE_USAGE [=n]
 as there are applets that have 20 or more options and
 [qwertyuiopasdfghjklzxcvbnm] is not better than simple [options].
  * [OPTIONS]


  If that looks OK to you, then I guess you do not like unified and
  simplified outputs.

 I just would like to point out that you rarely look at more than
 a help text at a time and this is maybe the because nobody
 complained so far.


How untrue... Actually, I looked at many applet's help text. Are you
seriously claiming that you use one applet in your life? I definitely think
that is untrue. Most people use several applets along their life, looking
at several help outputs. Do you think people only work on unifying and
simplifying things for fun without any practical merit?


  That is definitely not any KISS you are referring to. I
  would advise you to have a closer look than what you have apparently
 done.
 
  But let us imagine that for a second that what you are saying is true: it
  is still extremely cumbersome, error-prone, and nonsensical IMHO to write
  the help output yourself, either a rewrite for an existing or creating it
  for a new one.

 Most of the help texts out there are written exactly that way
 (by hand) and surprisingly the people out are able to get this
 job done in a coherent way..


What are you talking about? You yourself claimed in the end it is
incoherent. Look at frameworks like Qt/KDE/etc who deal with applications,
and how much they unify and simplify all this. No, do not hijack please
that they are bigger than busybox; that is _not_ the point here. The point
is that they have applets/applications that they want to behave
consistently.


  The applet author ought to be responsible only for the
  metadata, and no any more boilerplate.
 
  I think you just simply do not see the forest from the trees here, or the
  communication has just severely failed to bring the attention to the
 actual
  issue.
 

 I feel you would like to add even more trees by adding a machinery for
 a relatively simple task that has to deal with a lot of exceptions
 and later on needs to be maintained by somebody,


What-are-you-talking-about?

It is so relatively simple task that people got wrong? Are there silly
coders in busybox only? I guess not, so the process is just simply
error-prone

Re: [PATCH] taskset: support more than 64 cores

[Laszlo Papp]

On Tue, Aug 12, 2014 at 9:46 AM, Matthieu Ternisien d'Ouville 
matthieu@6wind.com wrote:

 On Tue, Aug 12, 2014 at 4:32 AM, Isaac Dunham ibid...@gmail.com wrote:

 On Mon, Aug 11, 2014 at 10:35:46PM +0100, Laszlo Papp wrote:
  On Mon, Aug 11, 2014 at 10:33 PM, Aaro Koskinen aaro.koski...@iki.fi
  wrote:
 
   Hi,
  
   On Mon, Aug 11, 2014 at 10:27:12PM +0100, Laszlo Papp wrote:
Show me one typical embedded system that is high-volume and has
 more than
64 cores. Even the full-fledged iphone tablets are not there and
 even if
they were, they would use complete utils rather than chopped most
   probably
anyhow. To me, this feature does not seem to fit busybox's goal
 unless
Apple, Samsung, etc were not notified about some recent boom in the
semiconductor industry.
  
   Embedded != consumer electronics.
  
 
  Not quite sure what you are trying to achieve, but you wrote embedded
  systems _word-by-word_, and I asked for one typical example with more
 than
  64 cores where busybox is so much needed.

 I'm pretty sure that Matthieu (the patch author) just said that 6wind
 (his company)
 uses or builds the sort of hardware where this is relevant.


 Yes, for example we have a TILEGX72 pci express card which does not have
 an hard drive. And even if there is a lot of RAM, we don't want to use it
 for the filesystem, the RAM is reserved for other function.


Right, thank you.

Please put this into the commit message next time. This is very important
information for accepting a change. Theoretical changes without practice
does not make any sense, so if you can show the practice, that makes a huge
difference and more sense.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 1/2] vi: avoid spurious error if no filename is provided

[Laszlo Papp]

Do you know under which circumstances it happens? Is this a regression or
does it require some special environment?

(I cannot reproduce it here with v1.20.2)


On Mon, Aug 18, 2014 at 1:59 PM, Ron Yorston r...@tigress.co.uk wrote:

 Since commit 32afd3a (vi: some simplifications) starting vi without a
 filename on the command line has resulted in an error message in the
 status line:  '(null)' Bad address.

 The commit removed a call to the file_size function which checked for a
 null filename.  This prevented file_insert (and hence open) from being
 called with a null filename.

 Signed-off-by: Ron Yorston r...@tigress.co.uk
 ---
  editors/vi.c |5 +++--
  1 files changed, 3 insertions(+), 2 deletions(-)

 diff --git a/editors/vi.c b/editors/vi.c
 index 24a9f60..9f55af1 100644
 --- a/editors/vi.c
 +++ b/editors/vi.c
 @@ -722,7 +722,7 @@ int vi_main(int argc, char **argv)
  /* will also update current_filename */
  static int init_text_buffer(char *fn)
  {
 -   int rc;
 +   int rc = -1;

 flush_undo_data();
 modified_count = 0;
 @@ -741,7 +741,8 @@ static int init_text_buffer(char *fn)
 free(current_filename);
 current_filename = xstrdup(fn);
 }
 -   rc = file_insert(fn, text, 1);
 +   if (fn != NULL)
 +   rc = file_insert(fn, text, 1);
 if (rc  0) {
 // file doesnt exist. Start empty buf with dummy line
 char_insert(text, '\n', NO_UNDO);
 --
 1.7.1

 ___
 busybox mailing list
 busybox@busybox.net
 http://lists.busybox.net/mailman/listinfo/busybox

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 1/2] vi: avoid spurious error if no filename is provided

[Laszlo Papp]

On Mon, Aug 18, 2014 at 2:13 PM, Ron Yorston r...@tigress.co.uk wrote:

 Laszlo Papp wrote:
 Do you know under which circumstances it happens? Is this a regression or
 does it require some special environment?
 
 (I cannot reproduce it here with v1.20.2)

 It's a regression on the master branch.  The commit is dated 2014-04-05.
 The problem isn't present in 1.20.2.


Indeed, I checked master (now) as well as  1.22.1 (earlier).

Could you please mention the regression in your commit message to make it
clearer?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: adduser/passwd: too long username

[Laszlo Papp]

On Mon, Aug 18, 2014 at 2:12 PM, Denys Vlasenko vda.li...@googlemail.com
wrote:

 On Sun, Aug 17, 2014 at 12:48 PM, Laszlo Papp lp...@kde.org wrote:
 
 
 
  On Tue, Aug 5, 2014 at 8:34 PM, Laszlo Papp lp...@kde.org wrote:
 
  On Tue, Aug 5, 2014 at 7:16 PM, Denys Vlasenko 
 vda.li...@googlemail.com
  wrote:
 
  On Mon, Aug 4, 2014 at 7:06 PM, Laszlo Papp lp...@kde.org wrote:
   sudo busybox adduser
  
  
 f
   passwd: unknown user
  
  
 f
  
   Yet, the user is created in /etc/shadow:
  
  
  
 f:!:16286:0:9:7:::
  
   This is at least one issue, but there is another here:
  
   sudo busybox deluser
  
  
 f
   deluser: unknown user
  
  
 f
 
  Both issues come from the same location in codebase:
  bb__pgsreader() parser drops lines which are longer than its buffer.
 
  Effectively, bbox ignores offending line in /etc/passwd.
 
   Could you please look into this and potentially fix it? Thanks in
   advance.
 
  Anyone willing to rewrite getpwnam API to use variable-sized malloced
  buffer?
 
 
  Is that a junior job?
 
 
  Denys, this fix was sent two weeks ago? Why have you not applied it until
  there is a better fix (if any)? This is still broken and results a system
  with potential stray users around.

 I'm having bad feelings about the fix along the lines of

 -#define PWD_BUFFER_SIZE 256
 -#define GRP_BUFFER_SIZE 256
 +#define PWD_BUFFER_SIZE 2*LOGIN_NAME_MAX+256
 +#define GRP_BUFFER_SIZE 2*LOGIN_NAME_MAX+256

 I fear that people (situations) strange enough to use names as long as

 f
 can easily use names thrice as long.


I do not follow. It is also completely inline with the desktop practice
that has existed for several decades now...


 From the API perspective, xmalloc_getpwnam(username) would be ideal.
 But it would require significant rework.


Exactly my point. I would be unhappy to keep patching my busybox locally
just because stray users can stay around on my system with the latest
busybox. My stance is usually applying changes that fix issues until there
are better approaches. Currently, I am not funded by anyone to work on this
nice design in full-time and I did provide a quick fix for the issue at
hand.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: adduser/passwd: too long username

[Laszlo Papp]

On Mon, Aug 18, 2014 at 2:37 PM, Denys Vlasenko vda.li...@googlemail.com
wrote:

 On Mon, Aug 18, 2014 at 3:17 PM, Laszlo Papp lp...@kde.org wrote:
   Denys, this fix was sent two weeks ago? Why have you not applied it
   until
   there is a better fix (if any)? This is still broken and results a
   system
   with potential stray users around.
 
  I'm having bad feelings about the fix along the lines of
 
  -#define PWD_BUFFER_SIZE 256
  -#define GRP_BUFFER_SIZE 256
  +#define PWD_BUFFER_SIZE 2*LOGIN_NAME_MAX+256
  +#define GRP_BUFFER_SIZE 2*LOGIN_NAME_MAX+256
 
  I fear that people (situations) strange enough to use names as long as
 
 
 f
  can easily use names thrice as long.
 
 
  I do not follow. It is also completely inline with the desktop practice
 that
  has existed for several decades now...
 
 
  From the API perspective, xmalloc_getpwnam(username) would be ideal.
  But it would require significant rework.
 
 
  Exactly my point. I would be unhappy to keep patching my busybox locally
  just because stray users can stay around on my system with the latest
  busybox. My stance is usually applying changes that fix issues until
 there
  are better approaches. Currently, I am not funded by anyone to work on
 this
  nice design in full-time and I did provide a quick fix for the issue at
  hand.

 How sure are you that a buffer of 3*256 is big enough?


It is big enough for the passwd file itself. The group file is another
issue... (luckily not affecting us).
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: adduser/passwd: too long username

[Laszlo Papp]

On Mon, Aug 18, 2014 at 2:40 PM, Laszlo Papp lp...@kde.org wrote:

 On Mon, Aug 18, 2014 at 2:37 PM, Denys Vlasenko vda.li...@googlemail.com
 wrote:

 On Mon, Aug 18, 2014 at 3:17 PM, Laszlo Papp lp...@kde.org wrote:
   Denys, this fix was sent two weeks ago? Why have you not applied it
   until
   there is a better fix (if any)? This is still broken and results a
   system
   with potential stray users around.
 
  I'm having bad feelings about the fix along the lines of
 
  -#define PWD_BUFFER_SIZE 256
  -#define GRP_BUFFER_SIZE 256
  +#define PWD_BUFFER_SIZE 2*LOGIN_NAME_MAX+256
  +#define GRP_BUFFER_SIZE 2*LOGIN_NAME_MAX+256
 
  I fear that people (situations) strange enough to use names as long as
 
 
 f
  can easily use names thrice as long.
 
 
  I do not follow. It is also completely inline with the desktop practice
 that
  has existed for several decades now...
 
 
  From the API perspective, xmalloc_getpwnam(username) would be ideal.
  But it would require significant rework.
 
 
  Exactly my point. I would be unhappy to keep patching my busybox locally
  just because stray users can stay around on my system with the latest
  busybox. My stance is usually applying changes that fix issues until
 there
  are better approaches. Currently, I am not funded by anyone to work on
 this
  nice design in full-time and I did provide a quick fix for the issue
 at
  hand.

 How sure are you that a buffer of 3*256 is big enough?


 It is big enough for the passwd file itself. The group file is another
 issue... (luckily not affecting us).


For the group issue, I do not see any other OK solution than reallocating,
but even that will have some upper limit of course unless you want to make
the system potentially crash by exhausting the complete memory.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Fix the addgroup help output

[Laszlo Papp]

On Mon, Aug 18, 2014 at 3:09 PM, tito farmat...@tiscali.it wrote:

 On Monday 18 August 2014 14:51:00 you wrote:
  On Mon, Aug 18, 2014 at 1:42 PM, tito farmat...@tiscali.it wrote:
 
   On Monday 18 August 2014 14:11:48 you wrote:
On Mon, Aug 18, 2014 at 1:07 PM, tito farmat...@tiscali.it wrote:
   
 On Monday 18 August 2014 10:50:18 Laszlo Papp wrote:
  On Mon, Aug 18, 2014 at 9:45 AM, walter harms wha...@bfs.de
 wrote:
 
  
  
   Am 17.08.2014 11:22, schrieb Denys Vlasenko:
On Fri, Aug 15, 2014 at 3:11 PM, lp...@archlinux.us wrote:
   
 Applets are defining the help display text on their own,
 and
   it is
different for different applets.
   
   
I don't see any obvious way to make it easier without
 significant
 bloat.
   
Pass a list of options, parameters and their explanations to
 a
 function
which builds help text? This will likely be bigger than the
 help
   text
(many pointers to small strings results in pointers taking
almost the same amount of space than strings!)
   
Even if you manage to avoid _that_ bloat somehow,
you can't bzip2-compress tiny bits of text.
We currently bzip2-compress our help text as one big blob,
with BIG savings in size - more than 50%
   
And then you discover that some applets really want a bit
of customization to the way how options are explained.
   
   
  
   Would that something for busybox.net/TODO section ?
   Verify that help text and option are in sync
  
 
  Would what for TODO? It should not never really be validated by a
   human
 in
  the first place. An applet author should just fill in the
 metadata.
   All
 the
  rest should be automatically handled IMHO. Otherwise, it is
   error-prone,
  will likely only cause the chaos that the applets have now with
   regards
 to
  this.
 

 HI,
 if you would like to take a look at busybox/docs/BusyBox.html
 there are displayed all help texts, most of them look ok to me,
 a few could need some fixes, so lets list them. It would be
 also useful to write down exactly to what standards we want
 to enforce. KISS.

   
I am not sure what you are looking at. They are  completely
 inconsistent.
They use at least three different schema for just the options, let
 alone
others:
  
   Which one do you prefer?
  
 
  It really does *not* matter what I prefer. The maintainer decides and it
 is
  _purely_ subjective, so there is not much point in telling what I prefer.
  That is the least significant point for me. Consistency is by far way
 above
  any personal style in my book.
 
 

 But we have to set some standard and telling what you thik is appropriate
 could be a start.


Again, it is purely subjective. There is nothing to be said there. It is
not productive to discuss whether I like blue or red and why. It is all
about personal preference, and the maintainer decides. This is _not_ my
point and I really mean it.


  
   This could be ok for indipendent options.
* [-fb]
   This could be ok for options that depend on each other
   or eventually [-f[b]]
* [-f] [b]
   THis could be ok for the short usage form  FEATURE_VERBOSE_USAGE [=n]
   as there are applets that have 20 or more options and
   [qwertyuiopasdfghjklzxcvbnm] is not better than simple [options].
* [OPTIONS]
  
  
If that looks OK to you, then I guess you do not like unified and
simplified outputs.
  
   I just would like to point out that you rarely look at more than
   a help text at a time and this is maybe the because nobody
   complained so far.
  
 
  How untrue... Actually, I looked at many applet's help text. Are you
  seriously claiming that you use one applet in your life? I definitely
 think
  that is untrue. Most people use several applets along their life, looking
  at several help outputs. Do you think people only work on unifying and
  simplifying things for fun without any practical merit?

 No I claim that I use them one at a time.


How is that relevant? The whole thread is about the whole applet-set, not
one. The whole point, and hence the whole suggestion is about the WHOLE
applet-set. It does not make much sense to talk about consistency in the
context of one separate applet.


That is definitely not any KISS you are referring to. I
would advise you to have a closer look than what you have apparently
   done.
   
But let us imagine that for a second that what you are saying is
 true: it
is still extremely cumbersome, error-prone, and nonsensical IMHO to
 write
the help output yourself, either a rewrite for an existing or
 creating it
for a new one.
  
   Most of the help texts out there are written exactly that way
   (by hand) and surprisingly the people out are able to get this
   job done in a coherent way..
  
 
  What

Re: Re: Re: missing format string in applets/usage_pod.c

[Laszlo Papp]

On Mon, Aug 18, 2014 at 7:50 PM, Cathey, Jim jcat...@ciena.com wrote:

 I don't like warnings on perfectly legal C.

 Promoting them to errors is even worse.



 That said, even with all this crap, provided

 that the strings are *constant* and don't contain

 substitutions the compiler has nothing to

 complain about.


It is not just about the warning and error, but the actual usage, too: if
you do not need a placeholder, why would you use printf in the first place?
It is not really meant for that. f/puts, on the other hand, were
established for that. While, it is perfectly legal C, like so many other
things that C sadly allows, it does not make much sense to me either. Note
that perfectly legal can be quite far from perfectly reasonable.




 -- Jim



 *From:* busybox-boun...@busybox.net [mailto:busybox-boun...@busybox.net] *On
 Behalf Of *Tanguy Pruvot
 *Sent:* Thursday, August 14, 2014 6:45 AM
 *To:* Laszlo Papp
 *Cc:* wald...@gmx.de; busybox
 *Subject:* Re: Re: Re: missing format string in applets/usage_pod.c



 its the same with bionic libc (arm)



 printf(test) is ok but not printf(buf) with char buf[] = test;
 printf(%s, buf) is ok



 2014-08-14 15:29 GMT+02:00 Laszlo Papp lp...@kde.org:

 Sorry, the list was left out at some point due to my mistake ... See below.



 On Thu, Aug 14, 2014 at 2:28 PM, Laszlo Papp lp...@kde.org wrote:

 On Thu, Aug 14, 2014 at 2:04 PM, wald...@gmx.de wrote:

 Hi Laszlo,


  Cannot reproduce with -Wall -Wpedantic -Wextra.

 Did you notice:

 applets/usage_pod.c:74:3: error: format not a string literal and no format
 arguments
 [-Werror=format-security]

 ?



 Hmm, yes, but I got used to that -Wall -Wpedantic -Wextra would cover the
 cases that need to be checked. I am not sure this option respects the C
 language though.



 Anyway, I guess I learnt something new today, thanks. :-)




 ___
 busybox mailing list
 busybox@busybox.net
 http://lists.busybox.net/mailman/listinfo/busybox



___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] taskset: support more than 64 cores

[Laszlo Papp]

On Mon, Aug 18, 2014 at 8:20 PM, Cathey, Jim jcat...@ciena.com wrote:

 Embedded != consumer electronics.

 Indeed.  Our products use Busybox,
 and with some of them it's not
 impossible to push the per-unit price
 into seven figures (USD).  They
 are flash-based, not disk-based,
 and file space is at a modest premium,
 which is why BB is being used to
 begin with.

 Only eight cores per CPU, though.


Then, it sounds like irrelevant in this thread since it is about CPU cores.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: adduser/passwd: too long username

[Laszlo Papp]

On Mon, Aug 18, 2014 at 7:37 PM, Natanael Copa nc...@alpinelinux.org
wrote:

 On Mon, 18 Aug 2014 15:37:46 +0200
 Denys Vlasenko vda.li...@googlemail.com wrote:

  On Mon, Aug 18, 2014 at 3:17 PM, Laszlo Papp lp...@kde.org wrote:
Denys, this fix was sent two weeks ago? Why have you not applied it
until
there is a better fix (if any)? This is still broken and results a
system
with potential stray users around.
  
   I'm having bad feelings about the fix along the lines of
  
   -#define PWD_BUFFER_SIZE 256
   -#define GRP_BUFFER_SIZE 256
   +#define PWD_BUFFER_SIZE 2*LOGIN_NAME_MAX+256
   +#define GRP_BUFFER_SIZE 2*LOGIN_NAME_MAX+256
  
   I fear that people (situations) strange enough to use names as long as
  
  
 f
   can easily use names thrice as long.
  
  
   I do not follow. It is also completely inline with the desktop
 practice that
   has existed for several decades now...
  
  
   From the API perspective, xmalloc_getpwnam(username) would be ideal.
   But it would require significant rework.
  
  
   Exactly my point. I would be unhappy to keep patching my busybox
 locally
   just because stray users can stay around on my system with the latest
   busybox. My stance is usually applying changes that fix issues until
 there
   are better approaches. Currently, I am not funded by anyone to work on
 this
   nice design in full-time and I did provide a quick fix for the issue
 at
   hand.
 
  How sure are you that a buffer of 3*256 is big enough?

 Not only that, it introduces a serious regression.

 I had a similar bug caused by the same underlying issue.
 Problem was that there was a limit on how many users you could add to a
 group. It is in fact an old issue:
 https://bugs.alpinelinux.org/issues/733

 I added a patch very similar the patch above, just slightly more
 conservative. I never liked this as it can cause unforseen consquences.

 http://git.alpinelinux.org/cgit/aports/commit/main/busybox?h=2.6-stableid=ab88f58f005a1177790e582e1f0171cc4ee5dcce


 However, this introduced a new issue, that caused login from console
 totally break:
 http://bugs.alpinelinux.org/issues/2838

 I never tested it bu I believe it breaks sulogin and httpd's password
 features as well. After grepping the sources I think solved all places
 with this patch:

 http://git.alpinelinux.org/cgit/aports/tree/main/busybox/pwdgrp-bufsize.patch?h=2.6-stableid=623c0906aa469523f04146e10b8ad7ab8cdc35f2

 *if* you want to bump the buf size you need bump it equally on all
 those places or you will get problems (if you go that route you should
 use a constant for those ofcourse - which might be a good idea anyways)

 I never sent those patches upstream because I don't think this is the
 correct fix. For Alpine Linux the correct fix was to use the libc
 implementation and a libc that handles this properly (musl libc)
 together with the patch I sent to this mailing list.

 http://lists.busybox.net/pipermail/busybox/2014-April/080809.html

 Commit message didn't say it but that patch is also needed to fix the
 username/groupsize issue - with glibc too I believe.

 -nc


I cannot reproduce most of your issues (all?), but I agree that one
constant could be used at several places.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Fix the addgroup help output

[Laszlo Papp]

On Sun, Aug 17, 2014 at 10:22 AM, Denys Vlasenko vda.li...@googlemail.com
wrote:

 On Fri, Aug 15, 2014 at 3:11 PM, lp...@archlinux.us wrote:

  Applets are defining the help display text on their own, and it is
 different for different applets.


 I don't see any obvious way to make it easier without significant bloat.

 Pass a list of options, parameters and their explanations to a function
 which builds help text? This will likely be bigger than the help text
 (many pointers to small strings results in pointers taking
 almost the same amount of space than strings!)

 Even if you manage to avoid _that_ bloat somehow,
 you can't bzip2-compress tiny bits of text.
 We currently bzip2-compress our help text as one big blob,
 with BIG savings in size - more than 50%

 And then you discover that some applets really want a bit
 of customization to the way how options are explained.


That is not the only way to avoid the current mess. You could also generate
these help texts during the build process by just passing options,
explanations, etc. The current way really is tad bad IMHO.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: adduser/passwd: too long username

[Laszlo Papp]

On Tue, Aug 5, 2014 at 8:34 PM, Laszlo Papp lp...@kde.org wrote:

 On Tue, Aug 5, 2014 at 7:16 PM, Denys Vlasenko vda.li...@googlemail.com
 wrote:

 On Mon, Aug 4, 2014 at 7:06 PM, Laszlo Papp lp...@kde.org wrote:
  sudo busybox adduser
 
 f
  passwd: unknown user
 
 f
 
  Yet, the user is created in /etc/shadow:
 
 
 f:!:16286:0:9:7:::
 
  This is at least one issue, but there is another here:
 
  sudo busybox deluser
 
 f
  deluser: unknown user
 
 f

 Both issues come from the same location in codebase:
 bb__pgsreader() parser drops lines which are longer than its buffer.

 Effectively, bbox ignores offending line in /etc/passwd.

  Could you please look into this and potentially fix it? Thanks in
 advance.

 Anyone willing to rewrite getpwnam API to use variable-sized malloced
 buffer?


 Is that a junior job?


Denys, this fix was sent two weeks ago? Why have you not applied it until
there is a better fix (if any)? This is still broken and results a system
with potential stray users around.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Fix the addgroup help output

[Laszlo Papp]

On Fri, Aug 15, 2014 at 1:31 PM, Denys Vlasenko vda.li...@googlemail.com
wrote:

 On Wed, Aug 13, 2014 at 2:25 PM, Laszlo Papp lp...@kde.org wrote:
  On Wed, Aug 13, 2014 at 12:54 PM, tito farmat...@tiscali.it wrote:
  On Wednesday 13 August 2014 12:13:10 Laszlo Papp wrote:
   On Wed, Aug 13, 2014 at 9:52 AM, Laszlo Papp lp...@kde.org wrote:
  
commit 55d6582d88470078cef09f52d1bc3c9c3f7fca6a
Author: Laszlo Papp lp...@kde.org
Date:   Wed Aug 13 09:48:08 2014 +0100
   
Fix the addgroup help output
   
Since the applet has two options, it is quite misleading to only
mention one in
the usage example. It should either use OPTIONS there or
 enumerate
the
possible
options. The latter seems to be more common in applet, so I
 picked
that one.
   
diff --git a/loginutils/addgroup.c b/loginutils/addgroup.c
index 22cd0e6..ce57459 100644
--- a/loginutils/addgroup.c
+++ b/loginutils/addgroup.c
@@ -11,7 +11,7 @@
  */
   
 //usage:#define addgroup_trivial_usage
-//usage:   [-g GID]  IF_FEATURE_ADDUSER_TO_GROUP([USER] )
GROUP
+//usage:   [-gS]  IF_FEATURE_ADDUSER_TO_GROUP([USER] )
GROUP
 //usage:#define addgroup_full_usage \n\n
 //usage:   Add a group  IF_FEATURE_ADDUSER_TO_GROUP(or add a
user
to a group) \n
 //usage: \n   -g GID  Group id
   
  
   Hmm, actually, I got some doubts whether this change is useful because
   while it fixes one issue, the applets seem randomly inconsistent, e.g.
   this
   is not holding any consistency with any other either:
  
   Usage: crond -fbS -l N -d N -L LOGFILE -c DIR
  
   At best, it is missing the brackets, but the applets are inconsistent
 at
   large overall, sadly, so I am not sure it is any improvement to fix
 this
   one on its own, and fixing all would be more intrusive than I care.
  
  Hi,
  maybe is more easy to understand as it shows which option needs
  a argument.
 
  +//usage:   [-g GID][-S]  IF_FEATURE_ADDUSER_TO_GROUP([USER] )
  GROUP
 
 
  Well, it is all about personal preferences, so whatever Denys wishes as
 the
  maintainer, but the more important objective issue is outlined above.

 I fixed help text by adding [-S].

  Why
  applets are doing this on their own is beyond my league other than
  hysterical raisins...

 I didn't understand this part.
 Why applets are doing /what/ on their own?


As mentioned in the thread above, applets are defining the help display
text on their own, and it is different for different applets. There is no
centralized common solution to this problem. Every little applet is doing
its own way as they wish.

Also, it will be difficult to change at many places if you happen to need
to.. centralization would rock here imho.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Fix the addgroup help output

[Laszlo Papp]

On Fri, Aug 15, 2014 at 5:12 PM, Denys Vlasenko vda.li...@googlemail.com
wrote:

 On Fri, Aug 15, 2014 at 3:11 PM, lp...@archlinux.us wrote:

  Applets are defining the help display text on their own, and it is
 different for different applets.

  Also, it will be difficult to change at many places if you happen to need
 to..

  centralization would rock here imho...


 Centralization used to rock, when all help texts
 were in include/usage.src.h (then named include/usage.h).

 It was easy to see all help texts in one place, but it was
 a complete nightmare to keep applet and its help text in sync:
 you needed to jump between two different files
 in order to check what every option does in code,
 and what help text says.

 People were routinely forgetting to update help text,
 because it was in separate file.

 Adding new applet needed editing many files.

 You can see all help texts in one place after a build:
 it's in generated file, include/usage.h;
 or you can just run auxiliary executable: applets/usage
 to have it on stdout.


That is not what I meant. What I meant is centralizing the formatting
output. That is, to have some common functions to which you pass your
options and the functions will output a standardized format.

I did not mean to put all the custom code into one place as a dump. That
would be indeed worse than the current, I agree.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Re: Re: missing format string in applets/usage_pod.c

[Laszlo Papp]

On Fri, Aug 15, 2014 at 6:31 PM, Rich Felker dal...@libc.org wrote:

 On Thu, Aug 14, 2014 at 11:58:56PM -0700, Isaac Dunham wrote:
  On Thu, Aug 14, 2014 at 04:15:50PM +0100, Laszlo Papp wrote:
   On Thu, Aug 14, 2014 at 4:05 PM, Rich Felker dal...@libc.org wrote:
  
On Thu, Aug 14, 2014 at 03:28:51PM +0100, Laszlo Papp wrote:
 On Thu, Aug 14, 2014 at 2:44 PM, Tanguy Pruvot 
 tanguy.pru...@gmail.com
 wrote:

  its the same with bionic libc (arm)
 
  printf(test) is ok but not printf(buf) with char buf[] =
 test;
  printf(%s, buf) is ok
 

 Yeah, I guess it is about personal preference. I personally do not
 like
the
 extended code just to make some smart option silent. IMHO, it
 makes the
 code needlessly longer by adding another small layer for the
 content of
the
 variable. If there is no need for formatting, why use printf in
 the first
 place and not fputs with stdout? IMHO, stdout is nicer than %s
 :-) On
the
 other hand, it is too bad that there is nothing between fputs and
 puts
 where you do not need to use the file descriptor and you do not
 get a new
 line either.
   
In principle stdout is more expensive than . stdout is a GOT
reference in PIC, and  is just PC-relative. For non-PIC it makes no
difference though.
   
  
   Then my PC is probably lying since the practice does seem to disagree
 with
   you, apparently as well as this post:
  
  
 http://stackoverflow.com/questions/17238784/c-puts-without-newline#comment24978835_17238840
  
   (and the one below, ok, it is not critical, but it performs better for
   others, too)
  
   Raw strings are also uglier IMHO than well-defined identifiers, like
   stdout, but that is personal taste, so I do not mind that one.
 
  printf() is almost always going to call vfprintf(stdout, fmt, ap);
  I've checked musl, uclibc, glibc, and netbsd; dietlibc is the only
  exception I've found, and it's ugly and does not have locking/retry/...
  newlib calls _vfprintf_r, which is almost the same concept.
  Meanwhile, printf has to do the formatting.
 
  In other words:
  Just because printf(%s, string) doesn't name stdout, don't assume that
  it doesn't use a reference to stdout.
  So an ACK for Laszlo Papp's recommendation of fputs().

 I was talking about code size overhead, not run time overhead. Of
 course there's a reference to stdout in printf. When you reference it
 again yourself, that's a second reference. It's not a big deal but
 apparently busybox folks care about this kind of micro-optimization...


Why not care if it is better and it has no disadvantage?

The size complaint does not matter since that size overhead is already in
the lib pointed out by Isaac. It will not automagically know to print to
stdout.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Re: Re: missing format string in applets/usage_pod.c

[Laszlo Papp]

Sorry, the list was left out at some point due to my mistake ... See below.


On Thu, Aug 14, 2014 at 2:28 PM, Laszlo Papp lp...@kde.org wrote:

 On Thu, Aug 14, 2014 at 2:04 PM, wald...@gmx.de wrote:

 Hi Laszlo,

  Cannot reproduce with -Wall -Wpedantic -Wextra.

 Did you notice:
 applets/usage_pod.c:74:3: error: format not a string literal and no
 format arguments
 [-Werror=format-security]
 ?


 Hmm, yes, but I got used to that -Wall -Wpedantic -Wextra would cover the
 cases that need to be checked. I am not sure this option respects the C
 language though.

 Anyway, I guess I learnt something new today, thanks. :-)

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Re: Re: missing format string in applets/usage_pod.c

[Laszlo Papp]

On Thu, Aug 14, 2014 at 2:44 PM, Tanguy Pruvot tanguy.pru...@gmail.com
wrote:

 its the same with bionic libc (arm)

 printf(test) is ok but not printf(buf) with char buf[] = test;
 printf(%s, buf) is ok


Yeah, I guess it is about personal preference. I personally do not like the
extended code just to make some smart option silent. IMHO, it makes the
code needlessly longer by adding another small layer for the content of the
variable. If there is no need for formatting, why use printf in the first
place and not fputs with stdout? IMHO, stdout is nicer than %s :-) On the
other hand, it is too bad that there is nothing between fputs and puts
where you do not need to use the file descriptor and you do not get a new
line either.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Re: Re: Re: missing format string in applets/usage_pod.c

[Laszlo Papp]

On Thu, Aug 14, 2014 at 3:36 PM, wald...@gmx.de wrote:

 Hi Laszlo,

  Yeah, I guess it is about personal preference. I personally do not like
 the
  extended code just to make some smart option silent. ...

 Based on your arguing it seems that you have NEVER heard about
 format string exploits which are around for 15 years?

 http://en.wikipedia.org/wiki/Uncontrolled_format_string


That is your interpretation, but what I am saying, _this code_ does not do
any exploit. Either way, please do not use printf. I have had a quick
measure, and fputs performs slightly better. The %s is an overhead in the
way.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Re: Re: missing format string in applets/usage_pod.c

[Laszlo Papp]

On Thu, Aug 14, 2014 at 4:05 PM, Rich Felker dal...@libc.org wrote:

 On Thu, Aug 14, 2014 at 03:28:51PM +0100, Laszlo Papp wrote:
  On Thu, Aug 14, 2014 at 2:44 PM, Tanguy Pruvot tanguy.pru...@gmail.com
  wrote:
 
   its the same with bionic libc (arm)
  
   printf(test) is ok but not printf(buf) with char buf[] = test;
   printf(%s, buf) is ok
  
 
  Yeah, I guess it is about personal preference. I personally do not like
 the
  extended code just to make some smart option silent. IMHO, it makes the
  code needlessly longer by adding another small layer for the content of
 the
  variable. If there is no need for formatting, why use printf in the first
  place and not fputs with stdout? IMHO, stdout is nicer than %s :-) On
 the
  other hand, it is too bad that there is nothing between fputs and puts
  where you do not need to use the file descriptor and you do not get a new
  line either.

 In principle stdout is more expensive than . stdout is a GOT
 reference in PIC, and  is just PC-relative. For non-PIC it makes no
 difference though.


Then my PC is probably lying since the practice does seem to disagree with
you, apparently as well as this post:

http://stackoverflow.com/questions/17238784/c-puts-without-newline#comment24978835_17238840

(and the one below, ok, it is not critical, but it performs better for
others, too)

Raw strings are also uglier IMHO than well-defined identifiers, like
stdout, but that is personal taste, so I do not mind that one.


 As for the topic at hand, I don't think it's necessary to make this
 change if the messages are DOCUMENTED as being format strings that are
 required not to contain any format specifiers, and there's a big
 warning to this effect in the source wherever they appear. printf
 allows format strings chosen (or even constructed) at runtime and the
 -W option to make this an error is bogus; at least it cannot be
 applied to code that was not written for a specific coding style that
 forbids such usage.


Well, the advantage of removing this security risk is that any string can
be passed without constraining the string anytime.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Re: Re: missing format string in applets/usage_pod.c

[Laszlo Papp]

Here is my benchmark just to show I am not speaking out of an imaginary
world:

#include stdio.h

int main()
{
const char *buff = foo;
int i = 0;
for (i; i  100; ++i)
// fputs(buff, stdout);
printf(%s, buff);
return 0;
}

Fputs:

real0m1.761s
user0m0.047s
sys 0m0.007s

Printf:

real0m2.811s
user0m0.097s
sys 0m0.020s



On Thu, Aug 14, 2014 at 4:15 PM, Laszlo Papp lp...@kde.org wrote:

 On Thu, Aug 14, 2014 at 4:05 PM, Rich Felker dal...@libc.org wrote:

 On Thu, Aug 14, 2014 at 03:28:51PM +0100, Laszlo Papp wrote:
  On Thu, Aug 14, 2014 at 2:44 PM, Tanguy Pruvot tanguy.pru...@gmail.com
 
  wrote:
 
   its the same with bionic libc (arm)
  
   printf(test) is ok but not printf(buf) with char buf[] = test;
   printf(%s, buf) is ok
  
 
  Yeah, I guess it is about personal preference. I personally do not like
 the
  extended code just to make some smart option silent. IMHO, it makes
 the
  code needlessly longer by adding another small layer for the content of
 the
  variable. If there is no need for formatting, why use printf in the
 first
  place and not fputs with stdout? IMHO, stdout is nicer than %s :-) On
 the
  other hand, it is too bad that there is nothing between fputs and puts
  where you do not need to use the file descriptor and you do not get a
 new
  line either.

 In principle stdout is more expensive than . stdout is a GOT
 reference in PIC, and  is just PC-relative. For non-PIC it makes no
 difference though.


 Then my PC is probably lying since the practice does seem to disagree with
 you, apparently as well as this post:


 http://stackoverflow.com/questions/17238784/c-puts-without-newline#comment24978835_17238840

 (and the one below, ok, it is not critical, but it performs better for
 others, too)

 Raw strings are also uglier IMHO than well-defined identifiers, like
 stdout, but that is personal taste, so I do not mind that one.


 As for the topic at hand, I don't think it's necessary to make this
 change if the messages are DOCUMENTED as being format strings that are
 required not to contain any format specifiers, and there's a big
 warning to this effect in the source wherever they appear. printf
 allows format strings chosen (or even constructed) at runtime and the
 -W option to make this an error is bogus; at least it cannot be
 applied to code that was not written for a specific coding style that
 forbids such usage.


 Well, the advantage of removing this security risk is that any string can
 be passed without constraining the string anytime.

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Fix the addgroup help output

[Laszlo Papp]

commit 55d6582d88470078cef09f52d1bc3c9c3f7fca6a
Author: Laszlo Papp lp...@kde.org
Date:   Wed Aug 13 09:48:08 2014 +0100

Fix the addgroup help output

Since the applet has two options, it is quite misleading to only
mention one in
the usage example. It should either use OPTIONS there or enumerate the
possible
options. The latter seems to be more common in applet, so I picked that
one.

diff --git a/loginutils/addgroup.c b/loginutils/addgroup.c
index 22cd0e6..ce57459 100644
--- a/loginutils/addgroup.c
+++ b/loginutils/addgroup.c
@@ -11,7 +11,7 @@
  */

 //usage:#define addgroup_trivial_usage
-//usage:   [-g GID]  IF_FEATURE_ADDUSER_TO_GROUP([USER] ) GROUP
+//usage:   [-gS]  IF_FEATURE_ADDUSER_TO_GROUP([USER] ) GROUP
 //usage:#define addgroup_full_usage \n\n
 //usage:   Add a group  IF_FEATURE_ADDUSER_TO_GROUP(or add a user
to a group) \n
 //usage: \n   -g GID  Group id
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Fix the addgroup help output

[Laszlo Papp]

Yeah, I see that you are confused, please read the commit message carefully:

The latter seems to be more common in applet, so I picked that one.

Have you even tried to run other applets to see their help output? To me,
it seems that you have not investigated anything before chiming in.

On Wed, Aug 13, 2014 at 10:55 AM, walter harms wha...@bfs.de wrote:



 Am 13.08.2014 10:52, schrieb Laszlo Papp:
  commit 55d6582d88470078cef09f52d1bc3c9c3f7fca6a
  Author: Laszlo Papp lp...@kde.org
  Date:   Wed Aug 13 09:48:08 2014 +0100
 
  Fix the addgroup help output
 
  Since the applet has two options, it is quite misleading to only
  mention one in
  the usage example. It should either use OPTIONS there or enumerate
 the
  possible
  options. The latter seems to be more common in applet, so I picked
 that
  one.
 
  diff --git a/loginutils/addgroup.c b/loginutils/addgroup.c
  index 22cd0e6..ce57459 100644
  --- a/loginutils/addgroup.c
  +++ b/loginutils/addgroup.c
  @@ -11,7 +11,7 @@
*/
 
   //usage:#define addgroup_trivial_usage
  -//usage:   [-g GID]  IF_FEATURE_ADDUSER_TO_GROUP([USER] )
 GROUP
  +//usage:   [-gS]  IF_FEATURE_ADDUSER_TO_GROUP([USER] ) GROUP
   //usage:#define addgroup_full_usage \n\n
   //usage:   Add a group  IF_FEATURE_ADDUSER_TO_GROUP(or add a user
  to a group) \n
   //usage: \n   -g GID  Group id
 


 I am confused.
 the original help said:
 [ -g GID]= option -g with Argument GID
 you version is:
 [ -gS ]  = option -g with Argument S  ??

 This was you intention ?

 re,
  wh


 ___
 busybox mailing list
 busybox@busybox.net
 http://lists.busybox.net/mailman/listinfo/busybox

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Fix the addgroup help output

[Laszlo Papp]

On Wed, Aug 13, 2014 at 9:52 AM, Laszlo Papp lp...@kde.org wrote:

 commit 55d6582d88470078cef09f52d1bc3c9c3f7fca6a
 Author: Laszlo Papp lp...@kde.org
 Date:   Wed Aug 13 09:48:08 2014 +0100

 Fix the addgroup help output

 Since the applet has two options, it is quite misleading to only
 mention one in
 the usage example. It should either use OPTIONS there or enumerate the
 possible
 options. The latter seems to be more common in applet, so I picked
 that one.

 diff --git a/loginutils/addgroup.c b/loginutils/addgroup.c
 index 22cd0e6..ce57459 100644
 --- a/loginutils/addgroup.c
 +++ b/loginutils/addgroup.c
 @@ -11,7 +11,7 @@
   */

  //usage:#define addgroup_trivial_usage
 -//usage:   [-g GID]  IF_FEATURE_ADDUSER_TO_GROUP([USER] ) GROUP
 +//usage:   [-gS]  IF_FEATURE_ADDUSER_TO_GROUP([USER] ) GROUP
  //usage:#define addgroup_full_usage \n\n
  //usage:   Add a group  IF_FEATURE_ADDUSER_TO_GROUP(or add a user
 to a group) \n
  //usage: \n   -g GID  Group id


Hmm, actually, I got some doubts whether this change is useful because
while it fixes one issue, the applets seem randomly inconsistent, e.g. this
is not holding any consistency with any other either:

Usage: crond -fbS -l N -d N -L LOGFILE -c DIR

At best, it is missing the brackets, but the applets are inconsistent at
large overall, sadly, so I am not sure it is any improvement to fix this
one on its own, and fixing all would be more intrusive than I care.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Fix the addgroup help output

[Laszlo Papp]

On Wed, Aug 13, 2014 at 12:54 PM, tito farmat...@tiscali.it wrote:

 On Wednesday 13 August 2014 12:13:10 Laszlo Papp wrote:
  On Wed, Aug 13, 2014 at 9:52 AM, Laszlo Papp lp...@kde.org wrote:
 
   commit 55d6582d88470078cef09f52d1bc3c9c3f7fca6a
   Author: Laszlo Papp lp...@kde.org
   Date:   Wed Aug 13 09:48:08 2014 +0100
  
   Fix the addgroup help output
  
   Since the applet has two options, it is quite misleading to only
   mention one in
   the usage example. It should either use OPTIONS there or enumerate
 the
   possible
   options. The latter seems to be more common in applet, so I picked
   that one.
  
   diff --git a/loginutils/addgroup.c b/loginutils/addgroup.c
   index 22cd0e6..ce57459 100644
   --- a/loginutils/addgroup.c
   +++ b/loginutils/addgroup.c
   @@ -11,7 +11,7 @@
 */
  
//usage:#define addgroup_trivial_usage
   -//usage:   [-g GID]  IF_FEATURE_ADDUSER_TO_GROUP([USER] )
 GROUP
   +//usage:   [-gS]  IF_FEATURE_ADDUSER_TO_GROUP([USER] ) GROUP
//usage:#define addgroup_full_usage \n\n
//usage:   Add a group  IF_FEATURE_ADDUSER_TO_GROUP(or add a
 user
   to a group) \n
//usage: \n   -g GID  Group id
  
 
  Hmm, actually, I got some doubts whether this change is useful because
  while it fixes one issue, the applets seem randomly inconsistent, e.g.
 this
  is not holding any consistency with any other either:
 
  Usage: crond -fbS -l N -d N -L LOGFILE -c DIR
 
  At best, it is missing the brackets, but the applets are inconsistent at
  large overall, sadly, so I am not sure it is any improvement to fix this
  one on its own, and fixing all would be more intrusive than I care.
 
 Hi,
 maybe is more easy to understand as it shows which option needs
 a argument.

 +//usage:   [-g GID][-S]  IF_FEATURE_ADDUSER_TO_GROUP([USER] )
 GROUP


Well, it is all about personal preferences, so whatever Denys wishes as the
maintainer, but the more important objective issue is outlined above. Why
applets are doing this on their own is beyond my league other than
hysterical raisins...
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] taskset: support more than 64 cores

[Laszlo Papp]

I wonder about the use case for this feature? I mean busybox is meant for
small systems in general, and =64 cores are not that small embedded
systems, at least not yet, yeah?


On Mon, Aug 11, 2014 at 1:58 PM, Matthieu Ternisien d'Ouville 
matthieu@6wind.com wrote:

 This patch adds support of more than 64 cores for taskset.

 Signed-off-by: Matthieu Ternisien d'Ouville matthieu@6wind.com
 ---
  miscutils/taskset.c |   39 +++
  1 file changed, 31 insertions(+), 8 deletions(-)

 diff --git a/miscutils/taskset.c b/miscutils/taskset.c
 index 4a9e323..2695e20 100644
 --- a/miscutils/taskset.c
 +++ b/miscutils/taskset.c
 @@ -129,16 +129,39 @@ int taskset_main(int argc UNUSED_PARAM, char **argv)
 }

 { /* Affinity was specified, translate it into cpu_set_t */
 -   unsigned i;
 -   /* Do not allow zero mask: */
 -   unsigned long long m = xstrtoull_range(aff, 0, 1,
 ULLONG_MAX);
 -   enum { CNT_BIT = CPU_SETSIZE  sizeof(m)*8 ? CPU_SETSIZE :
 sizeof(m)*8 };
 +   long cpu;
 +   int len = strlen(aff);
 +   const char *ptr = aff + len - 1;
 +   const char *prefix = ;
 +
 +   if (len  1  !memcmp(aff, 0x, 2)) {
 +   aff += 2;
 +   prefix = 0x;
 +   }

 CPU_ZERO(mask);
 -   for (i = 0; i  CNT_BIT; i++) {
 -   unsigned long long bit = (1ULL  i);
 -   if (bit  m)
 -   CPU_SET(i, mask);
 +   cpu = 0;
 +
 +   for (; ptr = aff; ptr--) {
 +   int val;
 +
 +   if (*ptr = '9'  *ptr = '0')
 +   val = *ptr - '0';
 +   else if (*ptr = 'f'  *ptr = 'a')
 +   val = *ptr + (10 - 'a');
 +   else
 +   bb_perror_msg_and_die(invalid mask %s%s,
 prefix, aff);
 +
 +   if (val  1)
 +   CPU_SET(cpu, mask);
 +   if (val  2)
 +   CPU_SET(cpu+1, mask);
 +   if (val  4)
 +   CPU_SET(cpu+2, mask);
 +   if (val  8)
 +   CPU_SET(cpu+3, mask);
 +
 +   cpu += 4;
 }
 }

 --
 1.7.10.4

 ___
 busybox mailing list
 busybox@busybox.net
 http://lists.busybox.net/mailman/listinfo/busybox

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] taskset: support more than 64 cores

[Laszlo Papp]

On Mon, Aug 11, 2014 at 10:15 PM, Aaro Koskinen aaro.koski...@iki.fi
wrote:

 Hi,

 On Mon, Aug 11, 2014 at 02:25:04PM +0100, Laszlo Papp wrote:
  I wonder about the use case for this feature? I mean busybox is meant for
  small systems in general, and =64 cores are not that small embedded
  systems, at least not yet, yeah?

 How you define small? Earlier you said using libc getconf is bloat
 (as nproc replacement) for multicore systems, but now you suggest
 that for cores  64 we should't care. Where do you draw the line?

 Embedded systems that have lots of RAM and CPUs/cores run-time may be
 still limited to having a boot flash of just few megs where the whole
 OS image needs to fit.


Show me one typical embedded system that is high-volume and has more than
64 cores. Even the full-fledged iphone tablets are not there and even if
they were, they would use complete utils rather than chopped most probably
anyhow. To me, this feature does not seem to fit busybox's goal unless
Apple, Samsung, etc were not notified about some recent boom in the
semiconductor industry.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] taskset: support more than 64 cores

[Laszlo Papp]

On Mon, Aug 11, 2014 at 10:33 PM, Aaro Koskinen aaro.koski...@iki.fi
wrote:

 Hi,

 On Mon, Aug 11, 2014 at 10:27:12PM +0100, Laszlo Papp wrote:
  Show me one typical embedded system that is high-volume and has more than
  64 cores. Even the full-fledged iphone tablets are not there and even if
  they were, they would use complete utils rather than chopped most
 probably
  anyhow. To me, this feature does not seem to fit busybox's goal unless
  Apple, Samsung, etc were not notified about some recent boom in the
  semiconductor industry.

 Embedded != consumer electronics.


Not quite sure what you are trying to achieve, but you wrote embedded
systems _word-by-word_, and I asked for one typical example with more than
64 cores where busybox is so much needed.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] taskset: support more than 64 cores

[Laszlo Papp]

On Mon, Aug 11, 2014 at 11:13 PM, Aaro Koskinen aaro.koski...@iki.fi
wrote:

 Hi,

 On Mon, Aug 11, 2014 at 10:35:46PM +0100, Laszlo Papp wrote:
  Not quite sure what you are trying to achieve, but you wrote embedded
  systems _word-by-word_, and I asked for one typical example with more
 than
  64 cores where busybox is so much needed.

 I'm not trying the limit busybox usage, and I don't think there are
 any typical embedded system. And Apple or Samsung are definitely not
 any reference. But if you are happy with their HW, then good for you.


Right, so you cannot bring up any valid and real use case for this, or do
not want despite the explicit clarification request, I take it. I do not
think theoretical changes should be added just because it is a good
technical challange. Valid use cases oughta be part of the commit message,
but at worst, comment explanation without even explicit request to be fair.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] taskset: support more than 64 cores

[Laszlo Papp]

On Mon, Aug 11, 2014 at 11:26 PM, Aaro Koskinen aaro.koski...@iki.fi
wrote:

 Hi,

 On Mon, Aug 11, 2014 at 11:19:34PM +0100, Laszlo Papp wrote:
  Right, so you cannot bring up any valid and real use case for this, or do
  not want despite the explicit clarification request, I take it. I do not
  think theoretical changes should be added just because it is a good
  technical challange. Valid use cases oughta be part of the commit
 message,
  but at worst, comment explanation without even explicit request to be
  fair.

 I think the use case is pretty clear: support more than 64 cores.


No, that is a *theory*.

Use case would be: here is the hardware that is in desperate need of
busybox because although it has 96/128/core, memory and flash are s
limited. I honestly cannot find any hardware like that, and I have been
looking for one now for an hour with google. Not even the supercomputing
mobile platforms are like that and as indicated, even there, having
complete utils is a not issue. It is not a use case to add support for some
random technical challenge. When some submits a change, that person is
expected to provide real case scenarios. Busybox could also add support for
everything that the complete utils can handle in the desktop world, but
that is against its original goals, luckily, since I would not see the
point of NIH there.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: adduser/passwd: too long username

[Laszlo Papp]

Yeah, mayhaps... Thanks for the prompt reply. I tried to debug the code,
but the busybox code here is a bit messy heavily abusing macros in C and
all that. It ain't easy I must confess!

For instance, see this section:

http://git.busybox.net/busybox/tree/libpwdgrp/pwd_grp.c#n227

The _source_ is included several times always getting a new meaning based
on some defines... Now, check this function:

 http://git.busybox.net/busybox/tree/libpwdgrp/pwd_grp_internal.c#n20

resultbuf will be always different depending on which include it is.
Since it is failing at the pw_name check in there, I wanted to print it
out, but no easy joy there as printing it like that will yield compilation
error when the file is being included next time from above.

Right, I thought instead of doing some #if a == b hackery, debugging
would be easier, BUT:

1) The default build is stripped, yuck!

2) The unstripped build cannot locate the symbols (*).

So, I am giving up on this for now; this is not the type of source code
that is so pleasant to work with. ;-)

Cheers, L.

* (gdb) b main

Breakpoint 2 at 0x407c71
(gdb) r
Starting program: /home/lpapp/Projects/busybox/busybox_unstripped
deluser 
f
warning: Could not load shared library symbols for linux-vdso.so.1.
Do you need set solib-search-path or set sysroot?

Breakpoint 2, 0x00407c71 in main ()
(gdb) list
No symbol table is loaded.  Use the file command.
(gdb)



On Mon, Aug 4, 2014 at 10:40 PM, tito farmat...@tiscali.it wrote:

 On Monday 04 August 2014 19:06:39 Laszlo Papp wrote:
  Hi,
 
  sudo busybox adduser
 
 f
  passwd: unknown user
 
 f
 
  Yet, the user is created in /etc/shadow:
 
 
 f:!:16286:0:9:7:::
 
  This is at least one issue, but there is another here:
 
  sudo busybox deluser
 
 f
  deluser: unknown user
 
 f
 
  So, basically, once you create that long username, you cannot remove it
  anymore with busybox and it keeps polluting your system.
 
  You have to gain other means to clean up! I am using this version over
 here:
 
  BusyBox v1.22.1 (2014-06-02 14:47:37 MSK) multi-call binary.
 
  Could you please look into this and potentially fix it? Thanks in
 advance.
 
  Cheers, L.
 
 Hi,
 if disabling libb's internal pwd/grp lib and by jumping through some hops
 it
 works for me:

 I need to add the group first as my system's groupadd command called by
 bb's adduser
 supports only groupnames of max 32 chars.

 ./busybox addgroup
 f
 ./busybox adduser
 f
 -G
 f
 Adding user
 `f'
 to group
 `f'
 ...
 Adding user
 f
 to group
 f
 Done.
 Enter new UNIX password:
 Retype new UNIX password:
 passwd: password updated successfully

 and then I could also delete it:

 ./busybox deluser
 f
 ./busybox delgroup
 f
 delgroup: unknown group
 f

 (deluser correctly deleted also the group)

 I suspect that the buffer size used in libbpwdgrp/pwd_grp.c is to small:

 #define PWD_BUFFER_SIZE 256
 #define GRP_BUFFER_SIZE 256

 as it is meant for the whole struct pw passwd (or struct gr group) fields
 which could be easily be bigger:

 grep * /etc/shadow | wc
   1   1 240

Re: adduser/passwd: too long username

[Laszlo Papp]

Here is my tested fix without being to debug the busybox code, so only code
reading and understanding were my friends:

commit 9610650b6ce2a4c1904f78a2dcdb47cad3d2e3d1
Author: Laszlo Papp lp...@kde.org
Date:   Tue Aug 5 11:42:24 2014 +0100

Allow 256 bytes long usernames as per Unix standards (usually)

diff --git a/libpwdgrp/pwd_grp.c b/libpwdgrp/pwd_grp.c
index 2060d78..368c252 100644
--- a/libpwdgrp/pwd_grp.c
+++ b/libpwdgrp/pwd_grp.c
@@ -23,8 +23,8 @@
 /**/
 /* Sizes for statically allocated buffers. */

-#define PWD_BUFFER_SIZE 256
-#define GRP_BUFFER_SIZE 256
+#define PWD_BUFFER_SIZE LOGIN_NAME_MAX+256
+#define GRP_BUFFER_SIZE LOGIN_NAME_MAX+256

 /**/
 /* Prototypes for internal functions. */



On Tue, Aug 5, 2014 at 11:22 AM, Laszlo Papp lp...@kde.org wrote:

 Yeah, mayhaps... Thanks for the prompt reply. I tried to debug the code,
 but the busybox code here is a bit messy heavily abusing macros in C and
 all that. It ain't easy I must confess!

 For instance, see this section:

 http://git.busybox.net/busybox/tree/libpwdgrp/pwd_grp.c#n227

 The _source_ is included several times always getting a new meaning based
 on some defines... Now, check this function:

  http://git.busybox.net/busybox/tree/libpwdgrp/pwd_grp_internal.c#n20

 resultbuf will be always different depending on which include it is.
 Since it is failing at the pw_name check in there, I wanted to print it
 out, but no easy joy there as printing it like that will yield compilation
 error when the file is being included next time from above.

 Right, I thought instead of doing some #if a == b hackery, debugging
 would be easier, BUT:

 1) The default build is stripped, yuck!

 2) The unstripped build cannot locate the symbols (*).

 So, I am giving up on this for now; this is not the type of source code
 that is so pleasant to work with. ;-)

 Cheers, L.

 * (gdb) b main

 Breakpoint 2 at 0x407c71
 (gdb) r
 Starting program: /home/lpapp/Projects/busybox/busybox_unstripped deluser 
 f
 warning: Could not load shared library symbols for linux-vdso.so.1.
 Do you need set solib-search-path or set sysroot?

 Breakpoint 2, 0x00407c71 in main ()
 (gdb) list
 No symbol table is loaded.  Use the file command.
 (gdb)



 On Mon, Aug 4, 2014 at 10:40 PM, tito farmat...@tiscali.it wrote:

 On Monday 04 August 2014 19:06:39 Laszlo Papp wrote:
  Hi,
 
  sudo busybox adduser
 
 f
  passwd: unknown user
 
 f
 
  Yet, the user is created in /etc/shadow:
 
 
 f:!:16286:0:9:7:::
 
  This is at least one issue, but there is another here:
 
  sudo busybox deluser
 
 f
  deluser: unknown user
 
 f
 
  So, basically, once you create that long username, you cannot remove it
  anymore with busybox and it keeps polluting your system.
 
  You have to gain other means to clean up! I am using this version over
 here:
 
  BusyBox v1.22.1 (2014-06-02 14:47:37 MSK) multi-call binary.
 
  Could you please look into this and potentially fix it? Thanks in
 advance.
 
  Cheers, L.
 
 Hi,
 if disabling libb's internal pwd/grp lib and by jumping through some hops
 it
 works for me:

 I need to add the group first as my system's groupadd command called by
 bb's adduser
 supports only groupnames of max 32 chars.

 ./busybox addgroup
 f
 ./busybox adduser
 f
 -G
 f
 Adding user
 `f'
 to group
 `f'
 ...
 Adding user
 f
 to group

Re: adduser/passwd: too long username

[Laszlo Papp]

I disagree. There is nothing to reject here. It fixes _my issue_ at hand.
If you want to fix other bugs in your system that you are facing in
reality, by all means, fix them in a separate change.

I definitely do not agree with dynamically allocated buffer for the simple
reasons of:

1) Slow.
2) You would still need a maximum anyway.
3) Needless code complication.

Let us keep things simple and good, you know the good old KISS principle.

P.S.: please do not bikeshed.


On Tue, Aug 5, 2014 at 1:17 PM, tito farmat...@tiscali.it wrote:

 On Tuesday 05 August 2014 12:46:08 you wrote:
  Here is my tested fix without being to debug the busybox code, so only
 code
  reading and understanding were my friends:
 
  commit 9610650b6ce2a4c1904f78a2dcdb47cad3d2e3d1
  Author: Laszlo Papp lp...@kde.org
  Date:   Tue Aug 5 11:42:24 2014 +0100
 
  Allow 256 bytes long usernames as per Unix standards (usually)
 
  diff --git a/libpwdgrp/pwd_grp.c b/libpwdgrp/pwd_grp.c
  index 2060d78..368c252 100644
  --- a/libpwdgrp/pwd_grp.c
  +++ b/libpwdgrp/pwd_grp.c
  @@ -23,8 +23,8 @@
   /**/
   /* Sizes for statically allocated buffers. */
 
  -#define PWD_BUFFER_SIZE 256
  -#define GRP_BUFFER_SIZE 256
  +#define PWD_BUFFER_SIZE LOGIN_NAME_MAX+256
  +#define GRP_BUFFER_SIZE LOGIN_NAME_MAX+256
 
   /**/
   /* Prototypes for internal functions. */
 

 Hi,
 yes I've thought also about this solution but rejected it because:

 1) there will not be enough space to hold the home dir if named the same
 as the user
 so you need at least:

 #define PWD_BUFFER_SIZE LOGIN_NAME_MAX+ 6 (for home) + LOGIN_NAME_MAX (or
 PATH_MAX?) + 256 (for the rest)

yet this might not be enough as in my passwd file I see a 101 char long
 passwd.
Add  the gecos fields (arbitrary lenght)  to it and the default shell
 and we are short on space again.

 2) same for #define GRP_BUFFER_SIZE LOGIN_NAME_MAX+256:
 here you need to take into account the group member list with
 an arbitrary number of members:


 #define GRP_BUFFER_SIZE LOGIN_NAME_MAX+256+ LOGIN_NAME_MAX*n:

 The only clean solution to fix it forever is dynamically allocated buffers
 created at the time you read in the line from the passwd files.

 Ciao,
 Tito


 P.S.: please don't top post.


 
  On Tue, Aug 5, 2014 at 11:22 AM, Laszlo Papp lp...@kde.org wrote:
 
   Yeah, mayhaps... Thanks for the prompt reply. I tried to debug the
 code,
   but the busybox code here is a bit messy heavily abusing macros in C
 and
   all that. It ain't easy I must confess!
  
   For instance, see this section:
  
   http://git.busybox.net/busybox/tree/libpwdgrp/pwd_grp.c#n227
  
   The _source_ is included several times always getting a new meaning
 based
   on some defines... Now, check this function:
  
http://git.busybox.net/busybox/tree/libpwdgrp/pwd_grp_internal.c#n20
  
   resultbuf will be always different depending on which include it is.
   Since it is failing at the pw_name check in there, I wanted to print it
   out, but no easy joy there as printing it like that will yield
 compilation
   error when the file is being included next time from above.
  
   Right, I thought instead of doing some #if a == b hackery, debugging
   would be easier, BUT:
  
   1) The default build is stripped, yuck!
  
   2) The unstripped build cannot locate the symbols (*).
  
   So, I am giving up on this for now; this is not the type of source code
   that is so pleasant to work with. ;-)
  
   Cheers, L.
  
   * (gdb) b main
  
   Breakpoint 2 at 0x407c71
   (gdb) r
   Starting program: /home/lpapp/Projects/busybox/busybox_unstripped
 deluser
 f
   warning: Could not load shared library symbols for linux-vdso.so.1.
   Do you need set solib-search-path or set sysroot?
  
   Breakpoint 2, 0x00407c71 in main ()
   (gdb) list
   No symbol table is loaded.  Use the file command.
   (gdb)
  
  
  
   On Mon, Aug 4, 2014 at 10:40 PM, tito farmat...@tiscali.it wrote:
  
   On Monday 04 August 2014 19:06:39 Laszlo Papp wrote:
Hi,
   
sudo busybox adduser
   
  
 f
passwd: unknown user
   
  
 f
   
Yet, the user is created in /etc/shadow:
   
   
  
 f:!:16286:0:9:7:::
   
This is at least one issue, but there is another here:
   
sudo busybox deluser

Re: adduser/passwd: too long username

[Laszlo Papp]

On Tue, Aug 5, 2014 at 1:42 PM, tito farmat...@tiscali.it wrote:

 On Tuesday 05 August 2014 14:27:57 you wrote:
  I disagree. There is nothing to reject here. It fixes _my issue_ at hand.
  If you want to fix other bugs in your system that you are facing in
  reality, by all means, fix them in a separate change.
 
  I definitely do not agree with dynamically allocated buffer for the
 simple
  reasons of:
 
  1) Slow.
  2) You would still need a maximum anyway.
  3) Needless code complication.
 
  Let us keep things simple and good, you know the good old KISS principle.
 
  P.S.: please do not bikeshed.
 
 
  On Tue, Aug 5, 2014 at 1:17 PM, tito farmat...@tiscali.it wrote:
 
   On Tuesday 05 August 2014 12:46:08 you wrote:
Here is my tested fix without being to debug the busybox code, so
 only
   code
reading and understanding were my friends:
   
commit 9610650b6ce2a4c1904f78a2dcdb47cad3d2e3d1
Author: Laszlo Papp lp...@kde.org
Date:   Tue Aug 5 11:42:24 2014 +0100
   
Allow 256 bytes long usernames as per Unix standards (usually)
   
diff --git a/libpwdgrp/pwd_grp.c b/libpwdgrp/pwd_grp.c
index 2060d78..368c252 100644
--- a/libpwdgrp/pwd_grp.c
+++ b/libpwdgrp/pwd_grp.c
@@ -23,8 +23,8 @@
   
  /**/
 /* Sizes for statically allocated buffers. */
   
-#define PWD_BUFFER_SIZE 256
-#define GRP_BUFFER_SIZE 256
+#define PWD_BUFFER_SIZE LOGIN_NAME_MAX+256
+#define GRP_BUFFER_SIZE LOGIN_NAME_MAX+256
   
   
  /**/
 /* Prototypes for internal functions. */
   
  
   Hi,
   yes I've thought also about this solution but rejected it because:
  
   1) there will not be enough space to hold the home dir if named the
 same
   as the user
   so you need at least:
  
   #define PWD_BUFFER_SIZE LOGIN_NAME_MAX+ 6 (for home) + LOGIN_NAME_MAX
 (or
   PATH_MAX?) + 256 (for the rest)
  
  yet this might not be enough as in my passwd file I see a 101 char
 long
   passwd.
  Add  the gecos fields (arbitrary lenght)  to it and the default
 shell
   and we are short on space again.
  
   2) same for #define GRP_BUFFER_SIZE LOGIN_NAME_MAX+256:
   here you need to take into account the group member list with
   an arbitrary number of members:
  
  
   #define GRP_BUFFER_SIZE LOGIN_NAME_MAX+256+ LOGIN_NAME_MAX*n:
  
   The only clean solution to fix it forever is dynamically allocated
 buffers
   created at the time you read in the line from the passwd files.
  
   Ciao,
   Tito
  
  
   P.S.: please don't top post.
  
  
   
On Tue, Aug 5, 2014 at 11:22 AM, Laszlo Papp lp...@kde.org wrote:
   
 Yeah, mayhaps... Thanks for the prompt reply. I tried to debug the
   code,
 but the busybox code here is a bit messy heavily abusing macros in
 C
   and
 all that. It ain't easy I must confess!

 For instance, see this section:

 http://git.busybox.net/busybox/tree/libpwdgrp/pwd_grp.c#n227

 The _source_ is included several times always getting a new meaning
   based
 on some defines... Now, check this function:


 http://git.busybox.net/busybox/tree/libpwdgrp/pwd_grp_internal.c#n20

 resultbuf will be always different depending on which include it
 is.
 Since it is failing at the pw_name check in there, I wanted to
 print it
 out, but no easy joy there as printing it like that will yield
   compilation
 error when the file is being included next time from above.

 Right, I thought instead of doing some #if a == b hackery,
 debugging
 would be easier, BUT:

 1) The default build is stripped, yuck!

 2) The unstripped build cannot locate the symbols (*).

 So, I am giving up on this for now; this is not the type of source
 code
 that is so pleasant to work with. ;-)

 Cheers, L.

 * (gdb) b main

 Breakpoint 2 at 0x407c71
 (gdb) r
 Starting program: /home/lpapp/Projects/busybox/busybox_unstripped
   deluser
  
 f
 warning: Could not load shared library symbols for linux-vdso.so.1.
 Do you need set solib-search-path or set sysroot?

 Breakpoint 2, 0x00407c71 in main ()
 (gdb) list
 No symbol table is loaded.  Use the file command.
 (gdb)



 On Mon, Aug 4, 2014 at 10:40 PM, tito farmat...@tiscali.it
 wrote:

 On Monday 04 August 2014 19:06:39 Laszlo Papp wrote:
  Hi,
 
  sudo busybox adduser
 

  
 f
  passwd: unknown user
 

  
 f

Re: adduser/passwd: too long username

[Laszlo Papp]

On Tue, Aug 5, 2014 at 2:06 PM, tito farmat...@tiscali.it wrote:

 On Tuesday 05 August 2014 14:47:53 you wrote:
  On Tue, Aug 5, 2014 at 1:42 PM, tito farmat...@tiscali.it wrote:
 
   On Tuesday 05 August 2014 14:27:57 you wrote:
I disagree. There is nothing to reject here. It fixes _my issue_ at
 hand.
If you want to fix other bugs in your system that you are facing in
reality, by all means, fix them in a separate change.
   
I definitely do not agree with dynamically allocated buffer for the
   simple
reasons of:
   
1) Slow.
2) You would still need a maximum anyway.
3) Needless code complication.
   
Let us keep things simple and good, you know the good old KISS
 principle.
   
P.S.: please do not bikeshed.
   
   
On Tue, Aug 5, 2014 at 1:17 PM, tito farmat...@tiscali.it wrote:
   
 On Tuesday 05 August 2014 12:46:08 you wrote:
  Here is my tested fix without being to debug the busybox code, so
   only
 code
  reading and understanding were my friends:
 
  commit 9610650b6ce2a4c1904f78a2dcdb47cad3d2e3d1
  Author: Laszlo Papp lp...@kde.org
  Date:   Tue Aug 5 11:42:24 2014 +0100
 
  Allow 256 bytes long usernames as per Unix standards
 (usually)
 
  diff --git a/libpwdgrp/pwd_grp.c b/libpwdgrp/pwd_grp.c
  index 2060d78..368c252 100644
  --- a/libpwdgrp/pwd_grp.c
  +++ b/libpwdgrp/pwd_grp.c
  @@ -23,8 +23,8 @@
 
  
  /**/
   /* Sizes for statically allocated buffers. */
 
  -#define PWD_BUFFER_SIZE 256
  -#define GRP_BUFFER_SIZE 256
  +#define PWD_BUFFER_SIZE LOGIN_NAME_MAX+256
  +#define GRP_BUFFER_SIZE LOGIN_NAME_MAX+256
 
 
  
  /**/
   /* Prototypes for internal functions. */
 

 Hi,
 yes I've thought also about this solution but rejected it because:

 1) there will not be enough space to hold the home dir if named the
   same
 as the user
 so you need at least:

 #define PWD_BUFFER_SIZE LOGIN_NAME_MAX+ 6 (for home) +
 LOGIN_NAME_MAX
   (or
 PATH_MAX?) + 256 (for the rest)

yet this might not be enough as in my passwd file I see a 101
 char
   long
 passwd.
Add  the gecos fields (arbitrary lenght)  to it and the default
   shell
 and we are short on space again.

 2) same for #define GRP_BUFFER_SIZE LOGIN_NAME_MAX+256:
 here you need to take into account the group member list with
 an arbitrary number of members:


 #define GRP_BUFFER_SIZE LOGIN_NAME_MAX+256+ LOGIN_NAME_MAX*n:

 The only clean solution to fix it forever is dynamically allocated
   buffers
 created at the time you read in the line from the passwd files.

 Ciao,
 Tito


 P.S.: please don't top post.


 
  On Tue, Aug 5, 2014 at 11:22 AM, Laszlo Papp lp...@kde.org
 wrote:
 
   Yeah, mayhaps... Thanks for the prompt reply. I tried to debug
 the
 code,
   but the busybox code here is a bit messy heavily abusing
 macros in
   C
 and
   all that. It ain't easy I must confess!
  
   For instance, see this section:
  
   http://git.busybox.net/busybox/tree/libpwdgrp/pwd_grp.c#n227
  
   The _source_ is included several times always getting a new
 meaning
 based
   on some defines... Now, check this function:
  
  
   http://git.busybox.net/busybox/tree/libpwdgrp/pwd_grp_internal.c#n20
  
   resultbuf will be always different depending on which
 include it
   is.
   Since it is failing at the pw_name check in there, I wanted to
   print it
   out, but no easy joy there as printing it like that will yield
 compilation
   error when the file is being included next time from above.
  
   Right, I thought instead of doing some #if a == b hackery,
   debugging
   would be easier, BUT:
  
   1) The default build is stripped, yuck!
  
   2) The unstripped build cannot locate the symbols (*).
  
   So, I am giving up on this for now; this is not the type of
 source
   code
   that is so pleasant to work with. ;-)
  
   Cheers, L.
  
   * (gdb) b main
  
   Breakpoint 2 at 0x407c71
   (gdb) r
   Starting program:
 /home/lpapp/Projects/busybox/busybox_unstripped
 deluser

  
 f
   warning: Could not load shared library symbols for
 linux-vdso.so.1.
   Do you need set solib-search-path or set sysroot?
  
   Breakpoint 2, 0x00407c71 in main ()
   (gdb) list
   No symbol table is loaded.  Use the file command.
   (gdb)
  
  
  
   On Mon, Aug 4, 2014 at 10:40 PM, tito

Re: adduser/passwd: too long username

[Laszlo Papp]

On Tue, Aug 5, 2014 at 2:14 PM, Laszlo Papp lp...@kde.org wrote:




 On Tue, Aug 5, 2014 at 2:06 PM, tito farmat...@tiscali.it wrote:

 On Tuesday 05 August 2014 14:47:53 you wrote:
  On Tue, Aug 5, 2014 at 1:42 PM, tito farmat...@tiscali.it wrote:
 
   On Tuesday 05 August 2014 14:27:57 you wrote:
I disagree. There is nothing to reject here. It fixes _my issue_ at
 hand.
If you want to fix other bugs in your system that you are facing in
reality, by all means, fix them in a separate change.
   
I definitely do not agree with dynamically allocated buffer for the
   simple
reasons of:
   
1) Slow.
2) You would still need a maximum anyway.
3) Needless code complication.
   
Let us keep things simple and good, you know the good old KISS
 principle.
   
P.S.: please do not bikeshed.
   
   
On Tue, Aug 5, 2014 at 1:17 PM, tito farmat...@tiscali.it wrote:
   
 On Tuesday 05 August 2014 12:46:08 you wrote:
  Here is my tested fix without being to debug the busybox code,
 so
   only
 code
  reading and understanding were my friends:
 
  commit 9610650b6ce2a4c1904f78a2dcdb47cad3d2e3d1
  Author: Laszlo Papp lp...@kde.org
  Date:   Tue Aug 5 11:42:24 2014 +0100
 
  Allow 256 bytes long usernames as per Unix standards
 (usually)
 
  diff --git a/libpwdgrp/pwd_grp.c b/libpwdgrp/pwd_grp.c
  index 2060d78..368c252 100644
  --- a/libpwdgrp/pwd_grp.c
  +++ b/libpwdgrp/pwd_grp.c
  @@ -23,8 +23,8 @@
 
  
  /**/
   /* Sizes for statically allocated buffers. */
 
  -#define PWD_BUFFER_SIZE 256
  -#define GRP_BUFFER_SIZE 256
  +#define PWD_BUFFER_SIZE LOGIN_NAME_MAX+256
  +#define GRP_BUFFER_SIZE LOGIN_NAME_MAX+256
 
 
  
  /**/
   /* Prototypes for internal functions. */
 

 Hi,
 yes I've thought also about this solution but rejected it because:

 1) there will not be enough space to hold the home dir if named
 the
   same
 as the user
 so you need at least:

 #define PWD_BUFFER_SIZE LOGIN_NAME_MAX+ 6 (for home) +
 LOGIN_NAME_MAX
   (or
 PATH_MAX?) + 256 (for the rest)

yet this might not be enough as in my passwd file I see a 101
 char
   long
 passwd.
Add  the gecos fields (arbitrary lenght)  to it and the default
   shell
 and we are short on space again.

 2) same for #define GRP_BUFFER_SIZE LOGIN_NAME_MAX+256:
 here you need to take into account the group member list with
 an arbitrary number of members:


 #define GRP_BUFFER_SIZE LOGIN_NAME_MAX+256+ LOGIN_NAME_MAX*n:

 The only clean solution to fix it forever is dynamically allocated
   buffers
 created at the time you read in the line from the passwd files.

 Ciao,
 Tito


 P.S.: please don't top post.


 
  On Tue, Aug 5, 2014 at 11:22 AM, Laszlo Papp lp...@kde.org
 wrote:
 
   Yeah, mayhaps... Thanks for the prompt reply. I tried to
 debug the
 code,
   but the busybox code here is a bit messy heavily abusing
 macros in
   C
 and
   all that. It ain't easy I must confess!
  
   For instance, see this section:
  
   http://git.busybox.net/busybox/tree/libpwdgrp/pwd_grp.c#n227
  
   The _source_ is included several times always getting a new
 meaning
 based
   on some defines... Now, check this function:
  
  
   http://git.busybox.net/busybox/tree/libpwdgrp/pwd_grp_internal.c#n20
  
   resultbuf will be always different depending on which
 include it
   is.
   Since it is failing at the pw_name check in there, I wanted to
   print it
   out, but no easy joy there as printing it like that will yield
 compilation
   error when the file is being included next time from above.
  
   Right, I thought instead of doing some #if a == b hackery,
   debugging
   would be easier, BUT:
  
   1) The default build is stripped, yuck!
  
   2) The unstripped build cannot locate the symbols (*).
  
   So, I am giving up on this for now; this is not the type of
 source
   code
   that is so pleasant to work with. ;-)
  
   Cheers, L.
  
   * (gdb) b main
  
   Breakpoint 2 at 0x407c71
   (gdb) r
   Starting program:
 /home/lpapp/Projects/busybox/busybox_unstripped
 deluser

  
 f
   warning: Could not load shared library symbols for
 linux-vdso.so.1.
   Do you need set solib-search-path or set sysroot?
  
   Breakpoint 2, 0x00407c71 in main ()
   (gdb) list
   No symbol table is loaded.  Use the file command.
   (gdb

Re: adduser/passwd: too long username

[Laszlo Papp]

On Tue, Aug 5, 2014 at 3:06 PM, tito farmat...@tiscali.it wrote:

 On Tuesday 05 August 2014 15:21:49 you wrote:
  On Tue, Aug 5, 2014 at 2:14 PM, Laszlo Papp lp...@kde.org wrote:
 
  
  
  
   On Tue, Aug 5, 2014 at 2:06 PM, tito farmat...@tiscali.it wrote:
  
   On Tuesday 05 August 2014 14:47:53 you wrote:
On Tue, Aug 5, 2014 at 1:42 PM, tito farmat...@tiscali.it wrote:
   
 On Tuesday 05 August 2014 14:27:57 you wrote:
  I disagree. There is nothing to reject here. It fixes _my
 issue_ at
   hand.
  If you want to fix other bugs in your system that you are
 facing in
  reality, by all means, fix them in a separate change.
 
  I definitely do not agree with dynamically allocated buffer for
 the
 simple
  reasons of:
 
  1) Slow.
  2) You would still need a maximum anyway.
  3) Needless code complication.
 
  Let us keep things simple and good, you know the good old KISS
   principle.
 
  P.S.: please do not bikeshed.
 
 
  On Tue, Aug 5, 2014 at 1:17 PM, tito farmat...@tiscali.it
 wrote:
 
   On Tuesday 05 August 2014 12:46:08 you wrote:
Here is my tested fix without being to debug the busybox
 code,
   so
 only
   code
reading and understanding were my friends:
   
commit 9610650b6ce2a4c1904f78a2dcdb47cad3d2e3d1
Author: Laszlo Papp lp...@kde.org
Date:   Tue Aug 5 11:42:24 2014 +0100
   
Allow 256 bytes long usernames as per Unix standards
   (usually)
   
diff --git a/libpwdgrp/pwd_grp.c b/libpwdgrp/pwd_grp.c
index 2060d78..368c252 100644
--- a/libpwdgrp/pwd_grp.c
+++ b/libpwdgrp/pwd_grp.c
@@ -23,8 +23,8 @@
   

  
  /**/
 /* Sizes for statically allocated buffers. */
   
-#define PWD_BUFFER_SIZE 256
-#define GRP_BUFFER_SIZE 256
+#define PWD_BUFFER_SIZE LOGIN_NAME_MAX+256
+#define GRP_BUFFER_SIZE LOGIN_NAME_MAX+256
   
   

  
  /**/
 /* Prototypes for internal functions. */
   
  
   Hi,
   yes I've thought also about this solution but rejected it
 because:
  
   1) there will not be enough space to hold the home dir if
 named
   the
 same
   as the user
   so you need at least:
  
   #define PWD_BUFFER_SIZE LOGIN_NAME_MAX+ 6 (for home) +
   LOGIN_NAME_MAX
 (or
   PATH_MAX?) + 256 (for the rest)
  
  yet this might not be enough as in my passwd file I see a
 101
   char
 long
   passwd.
  Add  the gecos fields (arbitrary lenght)  to it and the
 default
 shell
   and we are short on space again.
  
   2) same for #define GRP_BUFFER_SIZE LOGIN_NAME_MAX+256:
   here you need to take into account the group member list
 with
   an arbitrary number of members:
  
  
   #define GRP_BUFFER_SIZE LOGIN_NAME_MAX+256+
 LOGIN_NAME_MAX*n:
  
   The only clean solution to fix it forever is dynamically
 allocated
 buffers
   created at the time you read in the line from the passwd
 files.
  
   Ciao,
   Tito
  
  
   P.S.: please don't top post.
  
  
   
On Tue, Aug 5, 2014 at 11:22 AM, Laszlo Papp lp...@kde.org
 
   wrote:
   
 Yeah, mayhaps... Thanks for the prompt reply. I tried to
   debug the
   code,
 but the busybox code here is a bit messy heavily abusing
   macros in
 C
   and
 all that. It ain't easy I must confess!

 For instance, see this section:


 http://git.busybox.net/busybox/tree/libpwdgrp/pwd_grp.c#n227

 The _source_ is included several times always getting a
 new
   meaning
   based
 on some defines... Now, check this function:



 http://git.busybox.net/busybox/tree/libpwdgrp/pwd_grp_internal.c#n20

 resultbuf will be always different depending on which
   include it
 is.
 Since it is failing at the pw_name check in there, I
 wanted to
 print it
 out, but no easy joy there as printing it like that will
 yield
   compilation
 error when the file is being included next time from
 above.

 Right, I thought instead of doing some #if a == b
 hackery,
 debugging
 would be easier, BUT:

 1) The default build is stripped, yuck!

 2) The unstripped build cannot locate the symbols (*).

 So, I am giving up on this for now; this is not the type
 of
   source
 code
 that is so pleasant to work with. ;-)

 Cheers, L.

 * (gdb) b main

 Breakpoint 2 at 0x407c71
 (gdb) r
 Starting program:
   /home

Re: adduser/passwd: too long username

[Laszlo Papp]

On Tue, Aug 5, 2014 at 3:13 PM, Laszlo Papp lp...@kde.org wrote:




 On Tue, Aug 5, 2014 at 3:06 PM, tito farmat...@tiscali.it wrote:

 On Tuesday 05 August 2014 15:21:49 you wrote:
  On Tue, Aug 5, 2014 at 2:14 PM, Laszlo Papp lp...@kde.org wrote:
 
  
  
  
   On Tue, Aug 5, 2014 at 2:06 PM, tito farmat...@tiscali.it wrote:
  
   On Tuesday 05 August 2014 14:47:53 you wrote:
On Tue, Aug 5, 2014 at 1:42 PM, tito farmat...@tiscali.it wrote:
   
 On Tuesday 05 August 2014 14:27:57 you wrote:
  I disagree. There is nothing to reject here. It fixes _my
 issue_ at
   hand.
  If you want to fix other bugs in your system that you are
 facing in
  reality, by all means, fix them in a separate change.
 
  I definitely do not agree with dynamically allocated buffer
 for the
 simple
  reasons of:
 
  1) Slow.
  2) You would still need a maximum anyway.
  3) Needless code complication.
 
  Let us keep things simple and good, you know the good old KISS
   principle.
 
  P.S.: please do not bikeshed.
 
 
  On Tue, Aug 5, 2014 at 1:17 PM, tito farmat...@tiscali.it
 wrote:
 
   On Tuesday 05 August 2014 12:46:08 you wrote:
Here is my tested fix without being to debug the busybox
 code,
   so
 only
   code
reading and understanding were my friends:
   
commit 9610650b6ce2a4c1904f78a2dcdb47cad3d2e3d1
Author: Laszlo Papp lp...@kde.org
Date:   Tue Aug 5 11:42:24 2014 +0100
   
Allow 256 bytes long usernames as per Unix standards
   (usually)
   
diff --git a/libpwdgrp/pwd_grp.c b/libpwdgrp/pwd_grp.c
index 2060d78..368c252 100644
--- a/libpwdgrp/pwd_grp.c
+++ b/libpwdgrp/pwd_grp.c
@@ -23,8 +23,8 @@
   

  
  /**/
 /* Sizes for statically allocated buffers. */
   
-#define PWD_BUFFER_SIZE 256
-#define GRP_BUFFER_SIZE 256
+#define PWD_BUFFER_SIZE LOGIN_NAME_MAX+256
+#define GRP_BUFFER_SIZE LOGIN_NAME_MAX+256
   
   

  
  /**/
 /* Prototypes for internal functions. */
   
  
   Hi,
   yes I've thought also about this solution but rejected it
 because:
  
   1) there will not be enough space to hold the home dir if
 named
   the
 same
   as the user
   so you need at least:
  
   #define PWD_BUFFER_SIZE LOGIN_NAME_MAX+ 6 (for home) +
   LOGIN_NAME_MAX
 (or
   PATH_MAX?) + 256 (for the rest)
  
  yet this might not be enough as in my passwd file I see a
 101
   char
 long
   passwd.
  Add  the gecos fields (arbitrary lenght)  to it and the
 default
 shell
   and we are short on space again.
  
   2) same for #define GRP_BUFFER_SIZE LOGIN_NAME_MAX+256:
   here you need to take into account the group member list
 with
   an arbitrary number of members:
  
  
   #define GRP_BUFFER_SIZE LOGIN_NAME_MAX+256+
 LOGIN_NAME_MAX*n:
  
   The only clean solution to fix it forever is dynamically
 allocated
 buffers
   created at the time you read in the line from the passwd
 files.
  
   Ciao,
   Tito
  
  
   P.S.: please don't top post.
  
  
   
On Tue, Aug 5, 2014 at 11:22 AM, Laszlo Papp 
 lp...@kde.org
   wrote:
   
 Yeah, mayhaps... Thanks for the prompt reply. I tried to
   debug the
   code,
 but the busybox code here is a bit messy heavily abusing
   macros in
 C
   and
 all that. It ain't easy I must confess!

 For instance, see this section:


 http://git.busybox.net/busybox/tree/libpwdgrp/pwd_grp.c#n227

 The _source_ is included several times always getting a
 new
   meaning
   based
 on some defines... Now, check this function:



 http://git.busybox.net/busybox/tree/libpwdgrp/pwd_grp_internal.c#n20

 resultbuf will be always different depending on which
   include it
 is.
 Since it is failing at the pw_name check in there, I
 wanted to
 print it
 out, but no easy joy there as printing it like that will
 yield
   compilation
 error when the file is being included next time from
 above.

 Right, I thought instead of doing some #if a == b
 hackery,
 debugging
 would be easier, BUT:

 1) The default build is stripped, yuck!

 2) The unstripped build cannot locate the symbols (*).

 So, I am giving up on this for now; this is not the type
 of
   source
 code
 that is so pleasant to work with. ;-)

 Cheers, L.

 * (gdb) b main

 Breakpoint

Re: adduser/passwd: too long username

[Laszlo Papp]

commit 980965767ef3ace983746ee25e92665b87d16755
Author: Laszlo Papp lp...@kde.org
Date:   Tue Aug 5 11:42:24 2014 +0100

Allow 256 bytes long usernames as per Unix standards (usually)

diff --git a/libpwdgrp/pwd_grp.c b/libpwdgrp/pwd_grp.c
index 2060d78..9e4424f 100644
--- a/libpwdgrp/pwd_grp.c
+++ b/libpwdgrp/pwd_grp.c
@@ -23,8 +23,8 @@
 /**/
 /* Sizes for statically allocated buffers. */

-#define PWD_BUFFER_SIZE 256
-#define GRP_BUFFER_SIZE 256
+#define PWD_BUFFER_SIZE 2*LOGIN_NAME_MAX+256
+#define GRP_BUFFER_SIZE 2*LOGIN_NAME_MAX+256

 /**/
 /* Prototypes for internal functions. */
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: adduser/passwd: too long username

[Laszlo Papp]

To challenge this a bit further for completeness: the other option would be
to limit the username length to 32 or 48 bytes rather than allowing the
whole 128-byte desktop-range. 256 does look like a bit excessive that
getconf returns:

getconf LOGIN_NAME_MAX
256

I find it hard to achieve in even scripts and console, let alone user
interfaces like small touch screen. Although, that is probably an entirely
UI design area...


On Tue, Aug 5, 2014 at 4:15 PM, Laszlo Papp lp...@kde.org wrote:

 commit 980965767ef3ace983746ee25e92665b87d16755
 Author: Laszlo Papp lp...@kde.org
 Date:   Tue Aug 5 11:42:24 2014 +0100

 Allow 256 bytes long usernames as per Unix standards (usually)

 diff --git a/libpwdgrp/pwd_grp.c b/libpwdgrp/pwd_grp.c
 index 2060d78..9e4424f 100644
 --- a/libpwdgrp/pwd_grp.c
 +++ b/libpwdgrp/pwd_grp.c
 @@ -23,8 +23,8 @@
  /**/
  /* Sizes for statically allocated buffers. */

 -#define PWD_BUFFER_SIZE 256
 -#define GRP_BUFFER_SIZE 256
 +#define PWD_BUFFER_SIZE 2*LOGIN_NAME_MAX+256
 +#define GRP_BUFFER_SIZE 2*LOGIN_NAME_MAX+256

  /**/
  /* Prototypes for internal functions. */

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: adduser/passwd: too long username

[Laszlo Papp]

 it will be / only in case the home dir does not exist at login time
 as the login program does chdir to the home


That is what the linked page also says, yes.


 dir else
 you would be locked out of a broken system (damaged partition
 mounted to /home,  /root folder corrupted on main file system).
 This is what I understand, not that you should put / in the passwd
 files.


I do not see how this backs up the futile /home/foo entry when you
actually do not create a home directory for foo intentionally. Have you
read my concerns and comments above? I think it is a mistake to skip those
without replying to them one-by-one because I find them valid and that is
why I wrote them, but I will reiterate:

* you would not know whether a system is broken if you see /home/foo in
/etc/passwd, but cannot find the corresponding home folder if you come
back to your server 1-2 years later or not even that long, but with many
users to keep in mind.

* if you create a new home directory with something different than
/home/foo, it would be confusing again.

* If we write a usermod alike applet with minimal needs, /home/foo even
loses its meaning completely, although its meaning is questionable even
without that IMHO.

Having said that, this is tangential a discussion, and would have deserved
its own thread. I personally believe could do a better job than the old gnu
tools here by dropping the artificial memory-waster default since it
provides no value. If you want to create /home/foo later, you ought to
aim for a tool that does get the job right for you, rather than manually
mkdir'ing and expecting that everything should just work.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: adduser/passwd: too long username

[Laszlo Papp]

On Tue, Aug 5, 2014 at 8:24 PM, tito farmat...@tiscali.it wrote:

 On Tuesday 05 August 2014 18:09:08 you wrote:
  To challenge this a bit further for completeness: the other option would
 be
  to limit the username length to 32 or 48 bytes rather than allowing the
  whole 128-byte desktop-range. 256 does look like a bit excessive that
  getconf returns:
 
  getconf LOGIN_NAME_MAX
  256
 
  I find it hard to achieve in even scripts and console, let alone user
  interfaces like small touch screen. Although, that is probably an
 entirely
  UI design area...
 
 
  On Tue, Aug 5, 2014 at 4:15 PM, Laszlo Papp lp...@kde.org wrote:
 
   commit 980965767ef3ace983746ee25e92665b87d16755
   Author: Laszlo Papp lp...@kde.org
   Date:   Tue Aug 5 11:42:24 2014 +0100
  
   Allow 256 bytes long usernames as per Unix standards (usually)
  
   diff --git a/libpwdgrp/pwd_grp.c b/libpwdgrp/pwd_grp.c
   index 2060d78..9e4424f 100644
   --- a/libpwdgrp/pwd_grp.c
   +++ b/libpwdgrp/pwd_grp.c
   @@ -23,8 +23,8 @@
  
  /**/
/* Sizes for statically allocated buffers. */
  
   -#define PWD_BUFFER_SIZE 256
   -#define GRP_BUFFER_SIZE 256
   +#define PWD_BUFFER_SIZE 2*LOGIN_NAME_MAX+256
   +#define GRP_BUFFER_SIZE 2*LOGIN_NAME_MAX+256
  
  
  /**/
/* Prototypes for internal functions. */
  
 

 Hi,
 even if we do it that way still the problem in group files with
 groups with multiple members remains unresolved:

 with more than 8 members (or even less) .


Sure, but that is a slightly different question, although having said that,
I am wondering how you would avoid setting a limit for the maximum length
somewhere anyway?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

adduser/passwd: too long username

[Laszlo Papp]

Hi,

sudo busybox adduser
f
passwd: unknown user
f

Yet, the user is created in /etc/shadow:

f:!:16286:0:9:7:::

This is at least one issue, but there is another here:

sudo busybox deluser
f
deluser: unknown user
f

So, basically, once you create that long username, you cannot remove it
anymore with busybox and it keeps polluting your system.

You have to gain other means to clean up! I am using this version over here:

BusyBox v1.22.1 (2014-06-02 14:47:37 MSK) multi-call binary.

Could you please look into this and potentially fix it? Thanks in advance.

Cheers, L.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Deluser: deleting the home folder

[Laszlo Papp]

Denys, could you please provide feedback here?

1) Do you appreciate the feature?

2) If yes, which option would you like to see for it?


On Wed, Jul 16, 2014 at 1:28 PM, Laszlo Papp lp...@kde.org wrote:

 This patch was sent to the mailing list two weeks ago without further
 comment from the maintainer...


 On Fri, Jul 4, 2014 at 11:15 AM, Laszlo Papp lp...@kde.org wrote:




 On Thu, Jul 3, 2014 at 10:28 PM, tito farmat...@tiscali.it wrote:

 On Thursday 03 July 2014 22:38:23 you wrote:
  On Thu, Jul 3, 2014 at 8:31 PM, tito farmat...@tiscali.it wrote:
 
   On Thursday 03 July 2014 14:51:11 you wrote:
On Thu, Jul 3, 2014 at 12:59 PM, tito farmat...@tiscali.it
 wrote:
   
 On Thursday 03 July 2014 13:03:46 Laszlo Papp wrote:
  On Thu, Jul 3, 2014 at 11:10 AM, Laszlo Papp lp...@kde.org
 wrote:
 
   commit 761fd153e340a14abccc0af89f2f6617faf2077f
   Author: Laszlo Papp lp...@kde.org
   Date:   Thu Jul 3 11:06:58 2014 +0100
  
   Add optional home directory removal support to deluser
  
   diff --git a/loginutils/deluser.c b/loginutils/deluser.c
   index e39ac55..67b744b 100644
   --- a/loginutils/deluser.c
   +++ b/loginutils/deluser.c
   @@ -11,9 +11,10 @@
 */
  
//usage:#define deluser_trivial_usage
   -//usage:   USER
   +//usage:   [-h] USER
//usage:#define deluser_full_usage \n\n
//usage:   Delete USER from the system
   +//usage:   \n-h   Remove the home directory
  
//usage:#define delgroup_trivial_usage
//usage:   IF_FEATURE_DEL_USER_FROM_GROUP([USER]
 )GROUP
   @@ -35,11 +36,15 @@ int deluser_main(int argc, char **argv)
   /* Name of shadow or gshadow file */
   const char *sfile;
   /* Are we deluser or delgroup? */
   +struct passwd *pw = 0;
  
 
  This could probably be one line below not to distract the
 comment and
  corresponding variable declaration. Although, ideally, this
 would
   need to
  go to the case 2 branch, but I did not want to introduce a
 new
   block
  there with re-indenting many lines. Also, do you prefer NULL
   instead of
  0?
 
  Let me know what the preferred style is...
 
  The patch is tested with and without -h and it works. The
 option
  selection is -h which reminds some people the canonical
 help,
   but on
  the contrary, this is also what is used for adduser to create
 the
   home
  directory, so I picked it up for being consistent. Again, let
 me know
 your
  preference ...
 

 Hi,
 couldn't we change -h as it conflicts with -h/--help and use -r
 as in
 --remove-home:

   
Well, I prefer consistency, otherwise it will become to
 effectively use
   the
applets. After all, if you do not type anything, you will get the
 help
output, or misuse it, so why would we bloat the applet code with
 that?
   
  
   Hi,
   where is the bloat in doing:
  
//usage:#define deluser_trivial_usage
   -//usage:   USER
   +//usage:   [-r] USER
//usage:#define deluser_full_usage \n\n
//usage:   Delete USER from the system
   +//usage:   \n-r   Remove the home directory
  
 
  That does not make sense to me, I am afraid. It might be possible
 later to
  remove other config data, too. It is probably not acceptable, thus it
 is
  not done so even on desktop.
 
 
   and
  
   int do_delhome = 0;
   if (getopt32(argv, r)  1) { ++argv; --argc; do_delhome = 1; }
  
   or maybe simply:
  
   int do_delhome = getopt32(argv, r):
   argc -= optind;
   argv += optind;
  
 
  This looks worse than a simple increment to me, but it is such a minor
  detail that I do not think it is too relevant.
 
  -h is nice and consistent. I do not know why you would want help
 option two
  when it only has one option. You would double the option number. It
 would
  be an overkill in this case.

 Hi,
 I want not to double the number of options I just suggest to use
 -r instead of -h because:


 As already replied, -r is not clear an option. I was thinking about -h
 and --remove-home in the beginning. I think anything else is bad choice
 because it is inconsistent with the rest of the world. I prefer local
 consistency within busybox, this I picked up -h, but if Denys would like to
 avoid that local consistency, I suggest --remove-home to at least have some
 consistency, namely with the desktop.


 1) -h is mostly used for help (with a few exceptions I am aware of).
 2)  on the desktop:
   a) deluser uses  --remove-home   ( Remove the home directory of
 the user and its mailspool)
   b) userdel uses  -r, --remove  (Files in the user's home directory
 will be removed along with the
   home directory
 itself and the user's mail spool.)
  therefore using -r would be consistent and logic to use