[PATCH v2] mount: mount--move with mtab support fix
Rob Landley
rob at landley.net
Tue Feb 22 13:38:27 UTC 2011
I thought Nokia was a subsidiary of Microsoft now? (Didn't all the
Linux guys quit?)
On 02/22/2011 05:51 AM, Roman Borisov wrote:
> > added new update_mtab_entry() function to update /etc/mtab when new mounted
> > entry is created by mount--move. In such case update_mtab_entry() is used in
> > mount.c/mount_it_now() function instead of addmntent().
> >
> > fixes: http://lists.busybox.net/pipermail/busybox/2011-January/074510.html
> >
> > v2: added #if ENABLE_FEATURE_MTAB_SUPPORT in functions declarations;
> > added ENABLE_FEATURE_CLEAN_UP condition check for cleaning
If this is only ever used in one place, why does it live in libbb
instead of in mount.c?
The erase_mtab() function was shared by mount and umount. This isn't.
> > Signed-off-by: Roman Borisov <ext-roman.borisov at nokia.com>
> > ---
> > include/libbb.h | 3 ++
> > libbb/mtab.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > util-linux/mount.c | 6 +++-
> > 3 files changed, 61 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/libbb.h b/include/libbb.h
> > index 587a5f9..7cbe0a1 100644
> > --- a/include/libbb.h
> > +++ b/include/libbb.h
> > @@ -1099,7 +1099,10 @@ extern void run_applet_no_and_exit(int a, char **argv) NORETURN FAST_FUNC;
> > extern int match_fstype(const struct mntent *mt, const char *fstypes) FAST_FUNC;
> > extern struct mntent *find_mount_point(const char *name, int subdir_too) FAST_FUNC;
> > #endif
> > +#if ENABLE_FEATURE_MTAB_SUPPORT
> > extern void erase_mtab(const char * name) FAST_FUNC;
> > +extern void update_mtab_entry(const struct mntent *mp) FAST_FUNC;
> > +#endif
No, the prototype doesn't get #ifdefed. This is so you can
conditionally call the code in something like:
if (ENABLE_FEATURE_MTAB_SUPPORT) erase_mtab(blah);
And then the compiler's dead code elimination deletes the reference
(this is why we have function-secitons and gc-sections in the compiler),
but the code is always syntax checked the same way and changing the
config doesn't introduce build breaks.
> > extern unsigned int tty_baud_to_value(speed_t speed) FAST_FUNC;
> > extern speed_t tty_value_to_baud(unsigned int value) FAST_FUNC;
> > #if ENABLE_DESKTOP
> > diff --git a/libbb/mtab.c b/libbb/mtab.c
> > index 22bff64..3af9379 100644
> > --- a/libbb/mtab.c
> > +++ b/libbb/mtab.c
> > @@ -52,4 +52,58 @@ void FAST_FUNC erase_mtab(const char *name)
> > } else if (errno != EROFS)
> > bb_perror_msg(bb_path_mtab_file);
> > }
> > +
> > +/*
> > +update_mtab_entry() is used to update entry in case of mount--move.
> > +we are looking for existing entries mnt_dir which is equal to mnt_fsname of
> > +input mntent and replace it by new one.
> > +*/
> > +void FAST_FUNC update_mtab_entry(const struct mntent *mp)
> > +{
> > + struct mntent *entries, *m;
> > + int i, count;
> > + FILE *mountTable;
> > +
> > + mountTable = setmntent(bb_path_mtab_file, "r");
> > + if (!mountTable) {
> > + bb_perror_msg(bb_path_mtab_file);
> > + return;
> > + }
> > +
> > + entries = NULL;
> > + count = 0;
> > + while ((m = getmntent(mountTable)) != 0) {
> > + entries = xrealloc_vector(entries, 3, count);
> > + entries[count].mnt_fsname = xstrdup(m->mnt_fsname);
> > + entries[count].mnt_dir = xstrdup(m->mnt_dir);
> > + entries[count].mnt_type = xstrdup(m->mnt_type);
> > + entries[count].mnt_opts = xstrdup(m->mnt_opts);
> > + entries[count].mnt_freq = m->mnt_freq;
> > + entries[count].mnt_passno = m->mnt_passno;
> > + count++;
> > + }
> > + endmntent(mountTable);
> > +
> > + mountTable = setmntent(bb_path_mtab_file, "w");
> > + if (mountTable) {
> > + for (i = 0; i < count; i++) {
> > + if (strcmp(entries[i].mnt_dir, mp->mnt_fsname) != 0)
> > + addmntent(mountTable, &entries[i]);
> > + else
> > + addmntent(mountTable, mp);
> > + }
> > + endmntent(mountTable);
> > + } else if (errno != EROFS)
> > + bb_perror_msg(bb_path_mtab_file);
> > +
> > + if (ENABLE_FEATURE_CLEAN_UP) {
> > + for (i = 0; i < count; i++) {
> > + free(entries[i].mnt_fsname);
> > + free(entries[i].mnt_dir);
> > + free(entries[i].mnt_type);
> > + free(entries[i].mnt_opts);
> > + }
> > + }
> > +}
> > #endif
There is still a situation where symlinking /etc/mtab to /proc/mounts
isn't the right thing to do? All these years after the introduction of
shared subtrees?
Isn't the /etc/mtab support broken legacy crap, to which you're patching
a single weird corner case out of the 30 different ways it's broken at
the _design_ level?
> > diff --git a/util-linux/mount.c b/util-linux/mount.c
> > index 0f213bb..c0c828b 100644
> > --- a/util-linux/mount.c
> > +++ b/util-linux/mount.c
> > @@ -524,8 +524,10 @@ static int mount_it_now(struct mntent *mp, long vfsflags, char *filteropts)
> > mp->mnt_freq = mp->mnt_passno = 0;
> >
> > // Write and close.
> > -
> > - addmntent(mountTable, mp);
> > + if (vfsflags & MS_MOVE)
> > + update_mtab_entry(mp);
> > + else
> > + addmntent(mountTable, mp);
> > endmntent(mountTable);
> > if (ENABLE_FEATURE_CLEAN_UP) {
> > free(mp->mnt_dir);
Oh _wow_ the mount code has code's gotten ugly. Dunno why I'm
bothering, but:
If you look at this bit up near the top:
#if ENABLE_FEATURE_MTAB_SUPPORT
#define USE_MTAB (!(option_mask32 & OPT_n))
#else
#define USE_MTAB 0
#endif
And then notice this chunk:
mtab:
if (USE_MTAB && !rc && !(vfsflags & MS_REMOUNT)) {
char *fsname;
(Don't ask me why the goto label is indented away from the left edge
like Kernighan intended.)
But anyway, the point is this is a larger version of the if (ENABLE)
trick above, meaning the prototype is going to be used to verify the
function call all the time, but this code will be if (0) and the
compiler will eliminiate it via dead code elimination in certain config
situations, due to the comparison against a compile-time constant.
By adding an #ifdef around the prototype, you introduce a compiler
warning when mtab is configured out (I.E. "the normal case"), for no
obvious reason. if (0) has_no_prototype();
Rob
More information about the busybox
mailing list