public inbox for util-linux@vger.kernel.org
 help / color / mirror / Atom feed
* Bug: libmount essentially undocumented
@ 2014-03-27 18:49 Werner Baumann
  2014-03-28  8:52 ` Karel Zak
  0 siblings, 1 reply; 5+ messages in thread
From: Werner Baumann @ 2014-03-27 18:49 UTC (permalink / raw)
  To: util-linux

While there is a documentation at
https://www.kernel.org/pub/linux/utils/util-linux/v2.24/libmount-docs/index.html
this documentation mainly consists of nice formatting with almost no
information about what the library functions are doing. Therefore the
library is essentially unusable (except for the maintainer of the
library).

Some examples:

* libmount Overview
"The libmount library is used to parse /etc/fstab, /etc/mtab
and /proc/self/mountinfo files, manage the mtab file, evaluate mount
options, etc."
That's all. Well, "etc" may be a good starting point.

* struct libmnt_context
This is a central object of the library, used by a zillion of
functions. While it is perfectly o.k. to hide the internals from the
public interface, it is not o.k. to have no description of this object
at all.
What kind of data will it hold (well, the context I know)?
What is its intended use? There is one example use case, without
explanation except:
"This code is similar to: mount -o
aaa,bbb,ccc=CCC,noatime,noexec /mnt/foo", nothing more.
Please not: it says "similar to", not "the same as". It's anyone's guess
what might be the difference.

* mnt_new_context ()
"struct libmnt_context * mnt_new_context (void);
 Returns : newly allocated mount context"
Nothing more. What's the initial state of the context. What values are
the mount options set to. The defaults from 'man mount'? All zeros? Is
all zeros the same as the defaults from 'man mount'?
What are necessary functions to apply before you can use the mount
context in a call of mnt_context_mount(), what is optional?

* mnt_context_prepare_mount ()
"int mnt_context_prepare_mount (struct libmnt_context *cxt);
 Prepare context for mounting, unnecessary for mnt_context_mount().
 cxt : context
 Returns : negative number on error, zero on success"
Is this all I have to do. How does this function know which file system
I want to mount?
More confusing: A inline comment in the nfs mount program (maintained
by the maintainer of libmount) says:
"The libmount strictly uses only options from fstab if running in restricted mode (suid, non-root user). This is done in mnt_context_prepare_mount() by default."
Why is this not mentioned in the documentation of
mnt_context_prepare_mount? And how does the library know whether it is
in "restricted mode"? mnt_context_apply_fstab too has no information on
this.

* mnt_context_set_options ()
"int mnt_context_set_options (struct libmnt_context *cxt, const char
*optstr);
 cxt : mount context
 optstr : comma delimited mount options
 Returns : 0 on success, negative number in case of error."
What will happen when this function is applied a second time? Will the
new options replace the old ones, or will the new options be merged
into the old ones? Naming the function "*_set_*" indicates replacement,
for merging I would expect a name with "_add_". But looking at the nfs
code it seems to add the new optons and not replace the old ones.

* By chance I noticed a new file on my system: "/var/run/mount/utab".
  Not worth mentioning? Funnily comments in the nfs code mention
  "/dev/.mount/utab" which I can't find on my system. Maybe udev will
  create it on occasion or is it just the same moved to another
  location?

I will stop the examples here. It's the same with almost all functions
in libmount. From the documentation you can't know what a function will
do, leave alone what preconditions must be met before applying a
function on the mount context.

I consider this bug serious because it renders the library unusable.

Cheers
Werner

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Bug: libmount essentially undocumented
  2014-03-27 18:49 Bug: libmount essentially undocumented Werner Baumann
@ 2014-03-28  8:52 ` Karel Zak
  2014-03-28 16:32   ` Dale R. Worley
  0 siblings, 1 reply; 5+ messages in thread
From: Karel Zak @ 2014-03-28  8:52 UTC (permalink / raw)
  To: Werner Baumann; +Cc: util-linux

On Thu, Mar 27, 2014 at 07:49:12PM +0100, Werner Baumann wrote:
> * mnt_context_prepare_mount ()
> "int mnt_context_prepare_mount (struct libmnt_context *cxt);
>  Prepare context for mounting, unnecessary for mnt_context_mount().
>  cxt : context
>  Returns : negative number on error, zero on success"
> Is this all I have to do. How does this function know which file system
> I want to mount?

I'm not sure that we need this kind of babysitting, it's obvious that
there is probably a function to specify a filesystem.

> More confusing: A inline comment in the nfs mount program (maintained
> by the maintainer of libmount) says:

I'm author not maintainer of the helper.

> "The libmount strictly uses only options from fstab if running in restricted mode (suid, non-root user). This is done in mnt_context_prepare_mount() by default."
> Why is this not mentioned in the documentation of
> mnt_context_prepare_mount? And how does the library know whether it
> is in "restricted mode"? mnt_context_apply_fstab too has no
> information on this.

See mnt_context_is_restricted() ...

> * mnt_context_set_options ()
> "int mnt_context_set_options (struct libmnt_context *cxt, const char
> *optstr);
>  cxt : mount context
>  optstr : comma delimited mount options
>  Returns : 0 on success, negative number in case of error."
> What will happen when this function is applied a second time? Will the
> new options replace the old ones, or will the new options be merged
> into the old ones? Naming the function "*_set_*" indicates replacement,

mnt_context_set_options()
mnt_context_append_options()

> for merging I would expect a name with "_add_". But looking at the nfs
> code it seems to add the new optons and not replace the old ones.

it replaces old one, read the code again

> * By chance I noticed a new file on my system: "/var/run/mount/utab".
>   Not worth mentioning? Funnily comments in the nfs code mention
>   "/dev/.mount/utab" which I can't find on my system. Maybe udev will
>   create it on occasion or is it just the same moved to another
>   location?

well, the comment in NFS ode is obsolete, it's "/var/run/mount/utab".

> I will stop the examples here. It's the same with almost all functions
> in libmount. From the documentation you can't know what a function will
> do, leave alone what preconditions must be met before applying a
> function on the mount context.
> 
> I consider this bug serious because it renders the library unusable.

I'm absolutely agree that the documentation is really not perfect,
unfortunately now I have another priorities than improve the
documentation. 

Anyway it's open source, you can send patches, the docs is generated
from source code comments.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Bug: libmount essentially undocumented
  2014-03-28  8:52 ` Karel Zak
@ 2014-03-28 16:32   ` Dale R. Worley
  2014-03-28 23:59     ` Sami Kerola
  0 siblings, 1 reply; 5+ messages in thread
From: Dale R. Worley @ 2014-03-28 16:32 UTC (permalink / raw)
  To: util-linux

> Anyway it's open source, you can send patches, the docs is generated
> from source code comments.

I've always held to the principle that the project isn't done until
you can use it reliably after reading the documentation *alone* (that
is, *without* having to read the code).  Otherwise, it's not a
professionally-done job.  Certainly, it's not something I'd want a
potential employer to find with my name attached to it.

Dale

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Bug: libmount essentially undocumented
  2014-03-28 16:32   ` Dale R. Worley
@ 2014-03-28 23:59     ` Sami Kerola
  2014-04-03 23:52       ` Phillip Susi
  0 siblings, 1 reply; 5+ messages in thread
From: Sami Kerola @ 2014-03-28 23:59 UTC (permalink / raw)
  To: Dale R. Worley; +Cc: util-linux

On 28 March 2014 16:32, Dale R. Worley <worley@alum.mit.edu> wrote:
>> Anyway it's open source, you can send patches, the docs is generated
>> from source code comments.
>
> I've always held to the principle that the project isn't done until
> you can use it reliably after reading the documentation *alone* (that
> is, *without* having to read the code).  Otherwise, it's not a
> professionally-done job.  Certainly, it's not something I'd want a
> potential employer to find with my name attached to it.

Looking the git log I have nearly 1000 commits to this project. Thank
you for pointing out I have done unprofessional job.

'When it comes to all free software the question "why didn't they ..."
always has a simple answer: "because you didn't". You have three
options: 1. contribute. 2. don't use it. 3. accept it the way it is.'
- hegborg

-- 
Sami Kerola
http://www.iki.fi/kerolasa/

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Bug: libmount essentially undocumented
  2014-03-28 23:59     ` Sami Kerola
@ 2014-04-03 23:52       ` Phillip Susi
  0 siblings, 0 replies; 5+ messages in thread
From: Phillip Susi @ 2014-04-03 23:52 UTC (permalink / raw)
  To: kerolasa, Dale R. Worley; +Cc: util-linux

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 03/28/2014 07:59 PM, Sami Kerola wrote:
> 'When it comes to all free software the question "why didn't they
> ..." always has a simple answer: "because you didn't". You have
> three options: 1. contribute. 2. don't use it. 3. accept it the way
> it is.'

You forgot option number 4) kindly ask the maintainers to fix it and
hope they choose to.


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.14 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBCgAGBQJTPfRVAAoJEI5FoCIzSKrw7k8H/3UNiK+n2tjKNtUktI5XCFlJ
BUqKK1XYohgB1B7wODaNLMwFN5S81rhD9oSYKY+XrcvDZBj4ykDeOaPNRWBJMxyA
aXYu3kAcLpPRq/cwrf59XUb0cXQMFNla/MwQ7T6+3dWm4p1/rNNiKmrMwEqBX7Tn
bPRCUrAE3d9PsLtI7p0rMzyObYcSHbslDiqOK6GSzWPpUoaag3ckrzqClpZykU/s
OyMABmBNabqwng8YA0cCobj0ovZj4KLxV2//9Bv/Gsulm0A+TthrfbjsD1dTa2mu
mGrh7hJfPPR4XbXgg0wB3ocUySrwQ+DyAHvIcf5uua4rgUznc7zyAd3HlD83FXg=
=mxM2
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-04-03 23:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-27 18:49 Bug: libmount essentially undocumented Werner Baumann
2014-03-28  8:52 ` Karel Zak
2014-03-28 16:32   ` Dale R. Worley
2014-03-28 23:59     ` Sami Kerola
2014-04-03 23:52       ` Phillip Susi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox