All of lore.kernel.org
 help / color / mirror / Atom feed
From: Leo Liang <ycliang@andestech.com>
To: Cyril Hrubis <chrubis@suse.cz>
Cc: ltp@lists.linux.it
Subject: Re: [LTP] [v2 2/2] lib/tst_pid.c: Increase PIDS_RESERVED to avoid fork failure.
Date: Thu, 16 Feb 2023 14:52:14 +0000	[thread overview]
Message-ID: <Y+5DHpOxWi0iP8H0@ubuntu01> (raw)
In-Reply-To: <Y+3tg68PkP7ce2eV@yuki>

Hi Cyril,

On Thu, Feb 16, 2023 at 09:46:59AM +0100, Cyril Hrubis wrote:
> Hi!
> > Just out of curiosity, is there any reason that we should do this in plain C ?
> > (Otherwise, we could drop this patchset and stay with the current implementation)
> 
> There are a few, calling random scripts from C is a bad practice
> overall.
> 
> Portabilitity may be one of the problems, there are several
> iimplementations of the basic UNIX utilities for Linux eg. coreutils,
> busybox, toybox, etc. These implemtations are subtly incompatible, not
> all commandline options are supported and so on. And for the busybox and
> toybox some options can be disabled at a compile time. We leaned that
> sometimes you have to double check if the functionality available and
> most of the time the end result is that it's just easier to rewrite the
> code in C.
> 
> We also have rule to make tests as self contained as possible, which
> simplifies debugging. One of the problems is that we do not have the
> environment the shell code runs in under control, we had a few test
> failing for non-standard settings of the LANG variables.
> 
> In this case the code is reasonably simple, so it will be less likely to
> be problematic, however I would stil lean towards replacing it with C
> code.
> 
> tl;dr Calling shell code from C programs makes things less predictable
>       and possibly unstable.
> 

Understood! Thank you for the detailed explanation!!
Will send a v3 patch ASAP in accordance with your advice!

Best regards,
Leo

> -- 
> Cyril Hrubis
> chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

  reply	other threads:[~2023-02-16 14:52 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-14 12:25 [LTP] [v2 1/2] lib/tst_pid.c: Count used pid by traversing /proc Leo Yu-Chi Liang
2023-02-14 12:25 ` [LTP] [v2 2/2] lib/tst_pid.c: Increase PIDS_RESERVED to avoid fork failure Leo Yu-Chi Liang
2023-02-14 13:37   ` Petr Vorel
2023-02-14 13:43     ` Cyril Hrubis
2023-02-14 14:17       ` Leo Liang
2023-02-14 14:26         ` Cyril Hrubis
2023-02-14 15:09           ` Cyril Hrubis
2023-02-16  5:40             ` Leo Liang
2023-02-16  8:46               ` Cyril Hrubis
2023-02-16 14:52                 ` Leo Liang [this message]
2023-02-16 22:59                   ` Petr Vorel
2023-02-14 14:14     ` Leo Liang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y+5DHpOxWi0iP8H0@ubuntu01 \
    --to=ycliang@andestech.com \
    --cc=chrubis@suse.cz \
    --cc=ltp@lists.linux.it \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.