Re: Pop function

[Adam Beneschan <adam@irvine.com>]

On Dec 14, 4:06 pm, "Rego, P." <pvr...@gmail.com> wrote:
> Hello,
>
> Given a list type
>    type T_List is
>       record
>          Next : access T_List;
>          Item : Integer;
>       end record;
>    T_List_Ptr is access T_List;
>
> Is it right to implement a pop function like the following? (Free is an Unchecked_Deallocation)
>    function Pop (Sender : access T_List) return Integer is
>       Current_Sender_Ptr : T_List_Ptr := T_List_Ptr (Sender);
>       Current_Item : constant T_List := Sender.Item;

that should be "constant Integer", right?

>    begin
>       if Sender /= null then
>          if Sender.Next /= null then
>             Current_Sender_Ptr := T_List_Ptr (Current_Sender_Ptr.Next);
>          end if;
>                  Free (Current_Sender_Ptr);
>          return Current_Item;
>       else
>          return 0;
>       end if;
>    end Pop;
>
> I mean, if I set Current_Sender_Ptr := T_List_Ptr (Current_Sender_Ptr.Next), it's
> equivalent to Sender := Sender.Next?

No.  Current_Sender_Ptr is a local variable.  When you set
Current_Sender_Ptr, you're changing the value of the local variable,
but that does not affect *anything* outside the Pop procedure.  Free
(Current_Sender_Ptr) will then set Current_Sender_Ptr to null.  Then
Pop will return and Current_Sender_Ptr will go away since it's a local
variable, so the work you've done of setting it to the "Next" value
will be lost.  If you call
   N := Pop (Sender);
assuming that Sender points to the head of the list, this call won't
change Sender at all.  After the call, Sender will point to an item
that has been deallocated, which is very, very bad.

What you're probably missing is that Sender is treated as an IN
parameter here, so it can't be modified.  You probably want to make it
an IN OUT parameter, which means that Pop can't be a function (up
through Ada 2005; I think that rule is changing in Ada 2012).  So I'd
do something like

   procedure Pop (List_Head : in out T_List_Ptr; Item : out Integer)
is ...

and then in Pop, at some point, List_Head := List_Head.Next.  Plus
you'll need to rearrange the code so that you don't try to access
Sender.Item before checking to make sure Sender isn't null, as Martin
pointed out.  (Also, it wouldn't be my style to reserve a special
Integer value to indicate an empty list.  It might be OK in some
applications, but it makes the code that calls Pop more difficult to
follow.)

                                -- Adam