[Pkg-alsa-devel] Bug#106244: #106244

[Thomas Hood]

On Fri, 2004-02-27 at 20:15, David B Harris wrote:
> I'm familiar with the policy, but I'll admit to having a hard time
> reading that section.

It _is_ complicated.


> The problem I see with doing it in the preinst is as follows:
> 
> 1. alsa-utils preinst runs, moves asound.state to /var
> 2. alsa-driver old postrm runs /etc/init.d/alsa stop, state gets stored
>    to /etc (the old alsactl is still on-disk)
> 3. alsa-utils finishes unpacking
> 4. alsa-driver new postinst runs /etc/init.d/alsa start, mixer settings
>    are restored from /var

This will not happen because the sequence would be

1. alsa-utils new-preinst upgrade oldver -- moves asound.state to /var
2. alsa-utils and alsa-base are unpacked
3. alsa-base old-postrm upgrade newver -- possibly does "alsactl store"
   alsa-utils old-postrm upgrade newver
4. alsa-base new-postinst configure oldver -- possibly "alsactl restore"
   alsa-utils new-postinst configure oldver

I assume here that alsa-base and alsa-utils are upgrade simultaneously.


> In this situation, we see two side-effects; there is now
> /etc/asound.state and /var/lib/alsa/asound.state (the first one being
> the last set of saved mixer settings), and the mixer settings have been
> restored from /var/lib/alsa/asound.state, so they're potentially
> out-of-date (perhaps muted).

I agree that this would be bad and we should make sure that this
does not happen.


> Like I said though, I do have a hard time reading and understanding that
> section. I should probably go through dpkg source at some point, but I
> don't have the time for it now. Is the above scenario possible?

I don't think it is possible.

On reading through alsa-base.postinst I noted down the following
problems with the code.

* Indentation is irregular -- sometimes tabs, sometimes spaces
  are used.

* Communicates with the user using 'echo' is deprecated.
  See policy 3.10.1.

* This code

    for i in $cards

  is misleading because i is traditionally used to hold integers.
  I suggest:

    for card in $cards

* I see no need to create more than one tempfile in the configure
  routine and I see no need for it to have more than one name.
  Instead of this:

    tempfile="$(tempfile)"
    echo stuff > "$tempfile"
    debconfbit="$(tempfile)"
    echo otherstuff > "$debconfbit"
    filetodumpto="$(tempfile)"
    cat "$debconfbit" /usr/share/alsa-base/modules-snippet.conf \
        "$tempfile" > "$filetodumpto"
    if condition ; then cp $filetodumpto /etc/alsa/modutils/1.0 ; fi
    rm -f "$tempfile" "$debconfbit" "$filetodumpto"

  you could just do this:

    filetodumpto="$(tempfile)"
    echo otherstuff > "$filetodumpto"
    cat /usr/share/alsa-base/modules-snippet.conf >> "$filetodumpto"
    echo stuff >> "$filetodumpto"
    if condition ; then mv "$filetodumpto" /etc/alsa/modutils/1.0 ;
    else rm -f "$filetodumpto" ; fi
   
* The comment about force_stop_modules_before_suspend in
  /etc/default/alsa is incorrect.  It says

    # Set as true if you want to unload alsa modules before
    # your system suspends.

  whereas the choices are actually between 'forcibly-unload-driver',
  'stop-procs' and 'none'.

* This test always fails:

    if [ -f /etc/alsa/modutils/1.0 -a ! -f /etc/alsa/modutils/1.0 ]

  so there is probably an error here.

* The comment:

    # Stolen from ripped out code to stop this fuckage once and for all.
    # Reason that debhelper is above: we want the driver running when
    # we try this.

  isn't very clear.  Furthermore it suggests that preceding debhelper
  code "runs" the driver, presumably meaning that it loads the driver.
  But the only preceding debhelper-installed code is the update-rc.d
  line installed by dh_installinit and this will not load any drivers.

--
Thomas