On 08/29/2012 07:52 PM, Richard Marko wrote:
> Required for reporter-ureport to work with kernel oopses.
> 
> Signed-off-by: Richard Marko <[email protected]>
> ---
>  abrt.spec.in                             |  1 +
>  src/plugins/Makefile.am                  |  3 ++-
>  src/plugins/abrt-action-save-kernel-data | 36 
> ++++++++++++++++++++++++++++++++
>  src/plugins/koops_event.conf             |  1 +
>  4 files changed, 40 insertions(+), 1 deletion(-)
>  create mode 100755 src/plugins/abrt-action-save-kernel-data
> 
> diff --git a/abrt.spec.in b/abrt.spec.in
> index a1c00ba..65a03d3 100644
> --- a/abrt.spec.in
> +++ b/abrt.spec.in
> @@ -571,6 +571,7 @@ gtk-update-icon-cache %{_datadir}/icons/hicolor 
> &>/dev/null || :
>  %endif
>  %{_bindir}/abrt-dump-oops
>  %{_bindir}/abrt-action-analyze-oops
> +%{_bindir}/abrt-action-save-kernel-data
>  %{_mandir}/man1/abrt-action-analyze-oops.1*
>  
>  %files addon-xorg
> diff --git a/src/plugins/Makefile.am b/src/plugins/Makefile.am
> index 675b1a7..8829c06 100644
> --- a/src/plugins/Makefile.am
> +++ b/src/plugins/Makefile.am
> @@ -4,7 +4,8 @@ bin_SCRIPTS = \
>      abrt-action-install-debuginfo \
>      abrt-action-analyze-core \
>      abrt-action-analyze-vmcore \
> -    abrt-action-list-dsos
> +    abrt-action-list-dsos \
> +    abrt-action-save-kernel-data
>  
>  bin_PROGRAMS = \
>      abrt-watch-log \
> diff --git a/src/plugins/abrt-action-save-kernel-data 
> b/src/plugins/abrt-action-save-kernel-data
> new file mode 100755
> index 0000000..84bd447
> --- /dev/null
> +++ b/src/plugins/abrt-action-save-kernel-data
> @@ -0,0 +1,36 @@
> +#!/bin/bash
> +#
> +# Save pkg_{name, arch, version, release} for kernel oopses.
> +#
> +# These files are required by reporter-ureporter (mandatory
> +# in uReport).
> +#
> +
> +test -f kernel || exit 1

Please make it emit a short message here.

> +
> +echo "Looking for kernel package"
> +kernel_package="kernel"
> +kernel_version="$( sed 's/ .*//' kernel )"
> +
> +if grep -q "PAE" kernel; then
> +        echo "Looking for PAE kernel"
> +        kernel_package="kernel-PAE"
> +        kernel_version="$( echo "$kernel_version" | sed 's/\.PAE//' )"
> +fi
> +
> +package="$( rpm -q "$kernel_package" | grep "$kernel_version" )"
> +if [ $? == 0 ]; then
> +        echo "Kernel package $package found"
> +        rpm -q --qf "%{name}\n" "$package" > pkg_name
> +        rpm -q --qf "%{arch}\n" "$package" > pkg_arch
> +        rpm -q --qf "%{version}\n" "$package" > pkg_version
> +        rpm -q --qf "%{release}\n" "$package" > pkg_release
> +        epoch="$( rpm -q --qf "%{epoch}" "$package" )"
> +        if [ "$epoch" == "(none)" ]; then
> +            echo "0" > pkg_epoch
> +        else
> +            echo "$epoch" > pkg_epoch
> +        fi
> +else
> +        echo "Kernel package not found, tried version $kernel_version"

Can we tell people who see the message what this error will cause?
Something along the lines
        echo "Can't find kernel package corresponding to '$kernel_version'."
        echo "Can't record package version data (pkg_version, pkg_release etc)."

Do we want to "exit 1" here?

> +fi

Formatting issue: the "else" part of this if is rather small.
Maybe it makes sense to rewrite it like this?

if [ $? != 0 ]; then
        echo "Can't find kernel package corresponding to '$kernel_version'."
        echo "Can't record package version data (pkg_version, pkg_release etc)."
        exit 0 (or 1?)
fi
echo "Kernel package $package found"
rpm -q --qf "%{name}\n" "$package" > pkg_name
rpm -q --qf "%{arch}\n" "$package" > pkg_arch
rpm -q --qf "%{version}\n" "$package" > pkg_version
rpm -q --qf "%{release}\n" "$package" > pkg_release
epoch="$( rpm -q --qf "%{epoch}" "$package" )"
[ "$epoch" = "(none)" ] && epoch=0
echo "$epoch" > pkg_epoch

Reply via email to