Re: [LEDE-DEV] [PATCH 1/3] busybox: enable flock by default

[Mathias Kresin]

18.12.2017 10:48, Roman Yeryomin:

On 2017-12-18 09:58, Mathias Kresin wrote:

17.12.2017 19:30, Roman Yeryomin:

This is needed for procd init script protection to work.
flock adds 4248 bytes to stripped busybox binary.

Signed-off-by: Roman Yeryomin <ro...@advem.lv>
---
  package/utils/busybox/Config-defaults.in | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/utils/busybox/Config-defaults.in
b/package/utils/busybox/Config-defaults.in
index 2a8d9dd397..6fc5093055 100644
--- a/package/utils/busybox/Config-defaults.in
+++ b/package/utils/busybox/Config-defaults.in
@@ -1497,7 +1497,7 @@ config BUSYBOX_DEFAULT_FINDFS
      default n
  config BUSYBOX_DEFAULT_FLOCK
      bool
-    default n
+    default y
  config BUSYBOX_DEFAULT_FDFLUSH
      bool
      default n

We have a custom (f)lock command in LEDE [0], which is used for
example during wireless detect.

I only had a brief lock at your patch series, but the existing lock
command might do the job.

I've no idea why we have a custom lock command instead of using the
busybox provided flock. But shipping two (f)lock implementations at
the same time seem to be a waste of flash space.

Mathias

[0]
https://git.lede-project.org/?p=source.git;a=blob;f=package/utils/busybox/patches/220-add_lock_util.patch;hb=60a39e8f5af7ed710c5c62b131fd9df6519b64e4

I think we should use stock flock instead of patching and adding another
custom wheel.
If something doesn't work in stock flock we should fix that and send the
fix upstream.

I had a closer look at the commit history to understand why we have a
custom (f)lock implementation. It is as simple as our custom lock dates
around 2006, while the busybox flock applet was added 2010.

At least I'm fine to switch to the upstream flock. As implied by using
the word "switch", our custom lock applet should be removed at the same
time.

Adding flock and procd_lock is intended to replace all init custom locks
and others.
(and fix the racing problem once and for all)

package/network/config/ltq-vdsl-app/files/dsl_cpe_pipe.sh
package/base-files/files/lib/preinit/30_failsafe_wait
package/base-files/files/lib/preinit/40_run_failsafe_hook
package/base-files/files/sbin/sysupgrade

Most likely there are more scripts using lock.

Mathias

Locks aren't used only for init scripts. Within the packages directory the lock applet is used in:

smime.p7s
Description: S/MIME Cryptographic Signature

_______________________________________________
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev