Hi Aaron,
On Thu, 2024-06-13 at 16:01 -0400, Aaron Merey wrote:
> I've attached a patch that adds man pages for a few libelf functions.
> These man pages were originally generated using ChatGPT (model
> GPT-4o). I edited each man page by hand to help ensure brevity and
> accuracy.
>
> Parts of the original descriptions generated by ChatGPT have been left
> as is. I've attached the original man pages generated by ChatGPT for
> comparison.
So the first question really is where did ChatGPT get this information
from? I see it produces Copyright statements, but it is clear to me
those aren't accurate. What is the copyright status of these generated
manual pages?
It looks like your edits are so extensive that you can claim your own
copyright on them. But make sure that your Signed-off-by is really
accurate. CONTRIBUTING says:
The sign-off is a simple line at the end of the explanation for the
patch, which certifies that you wrote it or otherwise have the right
to pass it on as a patch under an appropriate license. The rules are
pretty simple: if you can certify the below:
Developer's Certificate of Origin
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me,
and I have the right to submit the contribution under each
license indicated in, or otherwise designated as being
applicable to, the file.
(b) The contribution was provided directly to me by some other
person who certified (a), and I have not modified it.
(c) I understand and agree that the project and the
contribution are public and that a record of the
contribution (including all personal information I submit
with it, including my sign-off) is maintained indefinitely
and may be redistributed.
I see it also sometimes generates a HISTORY section with a bogus "first
appeared in..." line. We do have symbol versioning, so you can get some
of this information from the libelf/libelf.maps file. But note that the
symbol version isn't the same as the elfutils version (you can use git
to map them). There are only 8 different versions that introduced new
symbols.
In general I wonder how useful the description text generated by
chatgtp is. It looks very generic and I am a little worried it seems to
happily generate "descriptions" for none-exiting libelf functions.
> The prompt used to generate the man pages was:
> Write a linux man page for the function called <FUNCTION> in the open
> source project called elfutils.
I think it would be easier if you make the prompt less specific. How
about just asking for a template that you can then fill in yourself?
Write a template man page for the function called <FUNCTION> for the
libelf library in the elfutils project for the "libelf Programmer's
Manual"
> My goal is to eventually add man pages for all elfutils library functions.
> If the man pages in this patch are acceptable then I will use the above
> method to create the remaining man pages. I will post these to
> elfutils-devel in batches to avoid the need for reviewing very large
> individual patches.
Thanks. A couple of minor comments below.
> From 920a9ac8875d82e8014bf5f3afa6d85dbf00b6cb Mon Sep 17 00:00:00 2001
> From: Aaron Merey <ame...@redhat.com>
> Date: Thu, 13 Jun 2024 15:20:03 -0400
> Subject: [PATCH] Add man pages for some libelf functions
>
> Add man pages for elf32_getehdr.3, elf64_getehdr.3, elf_errmsg.3,
> elf_errno.3 and elf_version.3
>
> Signed-off-by: Aaron Merey <ame...@redhat.com>
> ---
> doc/Makefile.am | 6 +++++-
> doc/elf32_getehdr.3 | 36 ++++++++++++++++++++++++++++++++++++
> doc/elf64_getehdr.3 | 36 ++++++++++++++++++++++++++++++++++++
> doc/elf_errmsg.3 | 26 ++++++++++++++++++++++++++
> doc/elf_errno.3 | 24 ++++++++++++++++++++++++
> doc/elf_version.3 | 23 +++++++++++++++++++++++
> 6 files changed, 150 insertions(+), 1 deletion(-)
> create mode 100644 doc/elf32_getehdr.3
> create mode 100644 doc/elf64_getehdr.3
> create mode 100644 doc/elf_errmsg.3
> create mode 100644 doc/elf_errno.3
> create mode 100644 doc/elf_version.3
>
> diff --git a/doc/Makefile.am b/doc/Makefile.am
> index 0c094af2..6a858c59 100644
> --- a/doc/Makefile.am
> +++ b/doc/Makefile.am
> @@ -18,7 +18,11 @@
> ## along with this program. If not, see <http://www.gnu.org/licenses/>.
> EXTRA_DIST = COPYING-GFDL README
> dist_man1_MANS=readelf.1 elfclassify.1 srcfiles.1
> -notrans_dist_man3_MANS=elf_update.3 elf_getdata.3 elf_clone.3 elf_begin.3
> +
> +notrans_dist_man3_MANS = elf_update.3 elf_getdata.3 elf_clone.3 elf_begin.3 \
> + elf32_getehdr.3 elf64_getehdr.3 elf_errmsg.3 elf_errno.3 \
> + elf_version.3
> +
Note that it doesn't apply anymore because I tweaked this in commit
80aeca1b836e ("doc: Always distribute all man pages") by adding some
comments before this line.
While you fix that merge conflict maybe also put one man page file on
one line. That makes it easier to insert new pages in the future.
> notrans_dist_man7_MANS=
> notrans_dist_man8_MANS=
> notrans_dist_man1_MANS=
> diff --git a/doc/elf32_getehdr.3 b/doc/elf32_getehdr.3
> new file mode 100644
> index 00000000..4dbb3886
> --- /dev/null
> +++ b/doc/elf32_getehdr.3
> @@ -0,0 +1,36 @@
> +.TH ELF32_GETEHDR 3
Could you use the same .TH template as the existing doc/elf*1 manpages
which say "Libelf" "Libelf Programmer's Manual"
> +.SH NAME
> +elf32_getehdr \- retrieve the ELF header for a 32-bit ELF descriptor
> +
> +.SH SYNOPSIS
> +.nf
> +#include <libelf.h>
> +
> +Elf32_Ehdr *elf32_getehdr(Elf *elf);
> +.fi
> +
> +.SH DESCRIPTION
> +.B elf32_getehdr
> +retrieves the ELF header for the given 32-bit ELF descriptor
> +.I elf.
> +The ELF header contains crucial metadata about the ELF file, such as the
> type, machine, version, entry point, program header table offset, and section
> header table offset.
Should it list the actual Elf32_Ehdr struct from elf.h here?
> +.SH PARAMETERS
> +.TP
> +.I elf
> +Pointer to the ELF descriptor from which to retrieve the ELF header.
> +
> +.SH RETURN VALUE
> +On success,
> +.B elf32_getehdr
> +returns a pointer to the
> +.B Elf32_Ehdr
> +structure. If the ELF descriptor is invalid or not a 32-bit ELF, it returns
> NULL and sets an error code retrievable by
> +.BR elf_errno (3).
> +
> +.SH SEE ALSO
> +.BR elf64_getehdr (3),
> +.BR elf_begin (3)
Add elf_errno (3) ?
> +.SH REPORTING BUGS
> +Report bugs to <elfutils-devel@sourceware.org> or
> https://sourceware.org/bugzilla/.
> diff --git a/doc/elf64_getehdr.3 b/doc/elf64_getehdr.3
> new file mode 100644
> index 00000000..acf9a125
> --- /dev/null
> +++ b/doc/elf64_getehdr.3
Same questions as for the elf32 variant.
> diff --git a/doc/elf_errmsg.3 b/doc/elf_errmsg.3
> new file mode 100644
> index 00000000..1031b581
> --- /dev/null
> +++ b/doc/elf_errmsg.3
> @@ -0,0 +1,26 @@
> +.TH ELF_ERRMSG 3
> +
> +.SH NAME
> +elf_errmsg \- return the error message string for a given libelf error code.
> +
> +.SH SYNOPSIS
> +.B #include <libelf.h>
> +
> +.BI "const char *elf_errmsg(int " err ");"
> +
> +.SH DESCRIPTION
> +The \fBelf_errmsg\fP function retrieves a human-readable string
> corresponding to the most recent error code set by a libelf library function.
> If \fIerr\fP is 0, the function returns the error message for the most recent
> error code. If \fIerr\fP is non-zero, the function returns the error message
> for the specified error code.
> +
> +.SH PARAMETERS
> +.TP
> +.I err
> +An \fIint\fP value specifying the error code. If this value is 0, the
> function returns the error message for the most recent error. If this value
> is non-zero, the function returns the error message for the specified error
> code.
>
Note that the libelf.h description describes the special argument value
-1:
/* Return error string for ERROR. If ERROR is zero, return error string
for most recent error or NULL is none occurred. If ERROR is -1 the
behaviour is similar to the last case except that not NULL but a legal
string is returned. */
> +.SH RETURN VALUE
> +The \fBelf_errmsg\fP function returns a string containing the error message.
> If there is no corresponding error message, it returns NULL.
> +
> +.SH SEE ALSO
> +.BR elf_errno (3)
> +
> +.SH REPORTING BUGS
> +Report bugs to <elfutils-devel@sourceware.org> or
> https://sourceware.org/bugzilla/.
> diff --git a/doc/elf_errno.3 b/doc/elf_errno.3
> new file mode 100644
> index 00000000..e25f5207
> --- /dev/null
> +++ b/doc/elf_errno.3
> @@ -0,0 +1,24 @@
> +.TH ELF_ERRNO 3
> +
> +.SH NAME
> +elf_errno \- retrieve the error code of the last failing libelf function
> call.
> +
> +.SH SYNOPSIS
> +.B #include <libelf.h>
> +
> +.BI "int elf_errno(void);"
> +
> +.SH DESCRIPTION
> +The \fBelf_errno\fP function retrieves the error code of the last failing
> libelf library function. This error code indicates the type of error that
> occurred during the failing function call.
Maybe include the description from libelf.h (specifically mention the
value is thread local):
/* Return error code of last failing function call. This value is kept
separately for each thread. */
> +
> +.SH PARAMETERS
> +This function does not take any parameters.
> +
> +.SH RETURN VALUE
> +The \fBelf_errno\fP function returns an integer representing the most recent
> error code. If no error has occurred, it returns 0. If an error occurred, the
> function returns a non-zero error code that corresponds to the specific
> error. Error codes can be passed to \fBelf_errmsg\fP in order to create a
> string describing the error.
> +
> +.SH SEE ALSO
> +.BR elf_errmsg (3)
> +
> +.SH REPORTING BUGS
> +Report bugs to <elfutils-devel@sourceware.org> or
> https://sourceware.org/bugzilla/.
> diff --git a/doc/elf_version.3 b/doc/elf_version.3
> new file mode 100644
> index 00000000..fd309ae1
> --- /dev/null
> +++ b/doc/elf_version.3
> @@ -0,0 +1,23 @@
> +.TH ELF_VERSION 3
> +
> +.SH NAME
> +elf_version \- set the ELF library version
> +
> +.SH SYNOPSIS
> +.B #include <libelf.h>
> +
> +.BI "unsigned int elf_version(unsigned int " version ");"
> +
> +.SH DESCRIPTION
> +The \fBelf_version\fP function sets the library's ELF version to the
> specified value. This function must be called before any other libelf
> functions are used.
> +
> +.SH PARAMETERS
> +.TP
> +.I version
> +An \fIunsigned int\fP value specifying the desired ELF library version. This
> is should be set to \fBEV_CURRENT\fP to indicate the current version of the
> library. At this time the only supported \fIversion\fP values are
> \fBEV_CURRENT\fP and \fBEV_NONE\fP.
> +
> +.SH RETURN VALUE
> +The \fBelf_version\fP function returns \fBEV_CURRENT\fP when \fIversion\fP
> is supported. If the specified version is not supported, it returns
> \fBEV_NONE\fP.
> +
> +.SH REPORTING BUGS
> +Report bugs to <elfutils-devel@sourceware.org> or
> https://sourceware.org/bugzilla/.
>
elf_version is kind of funny. EV_CURRENT is 1 and I don't believe we
will ever see another ELF version.
Cheers,
Mark
