-1

I'm trying to make a program that works like a very simple server in Perl.

The program itself is meant to work as a library catalogue, giving the user options of searching for books by title or author, and borrowing or returning books. The list of books is provided in a separate file.

Basically, it's supposed to take requests (files) from "Requests" folder, process them, and then give answers (also files) in "Answers" folder. After the process is over, it deletes the old requests and repeats the process (answers are deleted by the client after they are accepted).

It's meant to run as a daemon, but for some reason only the loop responsible for deleting the request files works in the background - the requests are not processed into answers, but are just deleted. Whenever a new request appears, it's almost immediately deleted.

I'm learning to use daemons and tried to emulate what is in this thread.

#!/usr/bin/perl
use warnings;
use strict;
use Proc::Daemon;

#FUNCTIONS DEFINTIONS
sub FindAuthor 
{
#try to find book by this author in the catalogue
}


sub FindTitle
{
#try to find book with this title in the catalogue
}


sub CheckIfCanBeReturned
{
#check if the book is borrowed and by whom
}

#attempt at daemonization
Proc::Daemon::Init;

my $continueWork = 1;
$SIG{TERM} = sub { $continueWork = 0 };

while ( $continueWork )
{

    sleep(2);
    my @RequestFilesArray = `ls /home/Ex5/Requests`;

    #list all requests currently in the Request folder
    for ( my $b = 0; $b < @RequestFilesArray; $b++)
    {
        my $cut = `printf "$RequestFilesArray[$b]" | wc -m`;
        $cut = $cut - 1;
        $RequestFilesArray[$b] = substr $RequestFilesArray[$b], 0, $cut;    
    }


    #the requests are formatted in such way,
    #that the first 2 letters indicate what the client wants to be done
    #and the rest is taken parameters used in the processing

    for (my $i = 0; $i < @RequestFilesArray; $i++)
    {
        my $UserRequest = `tail -1 Requests/$RequestFilesArray[$i]`;
            my $fix = `printf "$UserRequest" | wc -m`;
            $fix = $fix - 1;
            $UserRequest = substr $UserRequest, 0, $fix;

        my $RequestType = substr $UserRequest, 0, 2;
        my $RequestedValue = substr $UserRequest, 3;

        my $RequestNumber = $i;

        if ($RequestType eq "fa")
        {
            #FIND BY AUTHOR
            my @results = FindAuthor ($RequestedValue);

            my $filename = "/home/Ex5/Answers/" . $RequestFilesArray[$RequestNumber];

            open (my $answerFile, '>', $filename) or die "$!";

            for (my $a = 0; $a < @results; $a++)
            {
                print $answerFile $results[$a],"\n";
            }
            close $answerFile;

        }
        elsif ($RequestType eq "ft")
        {
            #FIND BY TITLE
            my @results = FindTitle ($RequestedValue);

            my $filename = "/home/Ex5/Answers/" . $RequestFilesArray[$RequestNumber];

            open ( my $answerFile, '>', $filename) or die "$!";

            for (my $a = 0; $a < @results; $a++)
            {
                print $answerFile $results[$a],"\n";
            }
            close $answerFile;

        }
        elsif ($RequestType eq "br")
        {
            #BOOK RETURN
            my $result = CheckIfCanBeReturned ($RequestedValue, $RequestFilesArray[$RequestNumber]);

            my $filename = "/home/Ex5/Answers/" . $RequestFilesArray[$RequestNumber];

            open ( my $answerFile, '>', $filename) or die "$!";
            print $answerFile $result;
            close $answerFile;
        }
        elsif ($RequestType eq "bb")
        {
            #BOOK BORROW
            my $result = CheckIfCanBeBorrowed ($RequestedValue, $RequestFilesArray[$RequestNumber]);

            my $filename = "/home/Ex5/Answers/" . $RequestFilesArray[$RequestNumber];

            open ( my $answerFile, '>', $filename) or die "$!";
            print $answerFile $result;
            close $answerFile;
        }
        else
        {
            print "something went wrong with this request";
        }
    }

    #deleting processed requests
    for ( my $e = 0; $e < @RequestFilesArray; $e++)
    {   
        my $removeReq = "/home/Ex5/Requests/" . $RequestFilesArray[$e];
        unlink $removeReq;
    }

#$continueWork =0;
}
Olekman
  • 11
  • 1
    Please provide a minimal, runable demonstration of the problem. – ikegami Jun 20 '17 at 20:59
  • Sending errors to `/dev/null` makes it rather hard to debug. You should start by fixing that! – ikegami Jun 20 '17 at 21:00
  • Try adding debugging `print` statements so you can see what the values of the variables are. – Barmar Jun 20 '17 at 21:16
  • Do you really need to call an external program to get the length of a string? Doesn't Perl have built-in functions for that? – Barmar Jun 20 '17 at 21:17
  • 1
    I see a couple of issues, but the biggest I see is `exec`ing calls to `tail`, `ls`, and `printf` (whaaa? Perl has a `printf`). I'd recommend, though it's not necessarily wrong, to use Perl's ability to iterate the array vs the traditional `for-loop` IE `for my $item ( @RequestFilesArray )`. I'd also suggest putting your `sleep(2)` at the end of the loop, not the front. I parrot what the others before me say. You should use `glob()` to get your file list. Please answer @ikegami's question. – Jim Jun 20 '17 at 21:35
  • Also, I've never used `Proc::Daemon` as far as I can remember, but according to http://search.cpan.org/~akreal/Proc-Daemon-0.23/lib/Proc/Daemon.pod, it doesn't look like you're using it right...at all. – Jim Jun 20 '17 at 22:51
  • 1
    @Jim, It's a correct and documented usage. That said, it would be more useful to instruct it to redirect STDERR to an actual file instead of the default `/dev/null`. – ikegami Jun 21 '17 at 04:41
  • Can I offer that [Example code for daemon stuff](https://stackoverflow.com/questions/26296206/perl-daemonize-with-child-daemons)might be worth a look? I'm not a big fan of `Proc::Daemon` as a first port of call - threading/forking isn't _that_ hard a concept to grok, and you really need to before doing much parallel code. – Sobrique Jun 21 '17 at 09:28

2 Answers2

4

You have written way too much code before attempting to test it. You have also started shell processes at every opportunity rather than learning the correct way to achieve things in Perl

The first mistake is to use ls to discover what jobs are waiting. ls prints multiple files per line, and you treat the whole of each line as a file name, using the bizarre printf "$RequestFilesArray[$b]" | wc -m instead of length $RequestFilesArray[$b]

Things only get worse after that

I suggest the following

  • Start again from scratch

  • Write your program in Perl. Perl isn't a shell language

  • Advance in very small increments, making sure that your code compiles and does what it is supposed to every three or four lines. It does wonders for the confidence to know that you're enhancing working code rather than creating a magical sequence of random characters

  • Learn how to debug. You appear to be staring at your code hoping for inspiration to strike in the manner of someone staring at their car engine in the hope of seeing why it won't start

  • Delete request files as part of processing the request, and only once the request has been processed and the answer file successfully written. It shouldn't be done in a separate loop

Borodin
  • 126,100
  • 9
  • 70
  • 144
0

Taking what you provided, here's some pseudocode I've devised for you that you can use as somewhat of a template. This is by NO MEANS exhaustive. I think the advice @Borodin gave is sound and prudent.

This is all untested and much of the new stuff is pseudocode. However, hopefully, there are some breadcrumbs from which to learn. Also, as I stated above, your use of Proc::Daemon::Init is suspect. At the very least, it is so minimally used that it is gobbling up whatever error(s) is/are occurring and you've no idea what's wrong with the script.

#!/usr/bin/perl -wl

use strict;
use File::Basename;
use File::Spec;
use Proc::Daemon;
use Data::Dumper;

# turn off buffering
$|++;

#FUNCTIONS DEFINTIONS
sub FindAuthor
{
#try to find book by this author in the catalogue
}


sub FindTitle
{
#try to find book with this title in the catalogue
}


sub CheckIfCanBeReturned
{
#check if the book is borrowed and by whom
}

sub tail
{
  my $file = shift;

# do work
}

sub find_by
{
  my $file = shift;
  my $val  = shift;
  my $by   = shift;
  my @results;
  my $xt   = 0;

# sanity check args
# do work

  if ( $by eq 'author' )
  {
    my @results = FindByAuthor(blah);
  }
  elsif ( $by eq 'blah' )
  {
    @results = blah();
  }
  #...etc 

  # really should use File::Spec IE
  my $filename = File::Spec->catfile('home', 'Ex5', 'Answers', $file);

  # might be a good idea to either append or validate you're not clobbering
  # an existent file here because this is currently clobbering.
  open (my $answerFile, '>', $filename) or die "$!";

  for ( @results )
  {
    print $answerFile $_,"\n";
  }
  close $answerFile;

  # have some error checking in place and set $xt to 1 if an error occurs
  return $xt;
}

#attempt at daemonization
# whatever this is is completely broken methinks.
#Proc::Daemon::Init;

my $continueWork++;
my $r_dir = '/home/user/Requests';

$SIG{TERM} = sub { $continueWork = 0 };

# going with pseudocode
while ( $continueWork )
{
  #list all requests currently in the Request folder
  my @RequestFilesArray = grep(/[^\.]/, <$r_dir/*>);

  #the requests are formatted in such way,
  #that the first 2 letters indicate what the client wants to be done
  #and the rest is taken parameters used in the processing

  for my $request_file ( @RequestFilesArray )
  {
    my $result    = 0;

    $request_file = basename($request_file);
    my $cut       = length($request_file) - 1;
    my $work_on   = substr $request_file, 0, $cut;

    my $UserRequest = tail($request_file);
    my $fix       = length($UserRequest) - 1;
     $UserRequest = substr $UserRequest, 0, $fix;

    my $RequestType = substr $UserRequest, 0, 2;
    my $RequestedValue = substr $UserRequest, 3;

    if ($RequestType eq "fa")
    {
      #FIND BY AUTHOR
      $result = find_by($request_file, $RequestedValue, 'author');
    }
    elsif ($RequestType eq "ft")
    {
      #FIND BY TITLE
      $result = find_by($request_file, $RequestedValue, 'title');
    }
    elsif ($RequestType eq "br")
    {
      #BOOK RETURN
      $result = CheckIfCanBeReturned ($RequestedValue, $request_file) or handle();
    }
    elsif ($RequestType eq "bb")
    {
      #BOOK BORROW
      $result = CheckIfCanBeBorrowed ($RequestedValue, $request_file) or handle();
    }
    else
    {
      print STDERR "something went wrong with this request";
    }
  }

  #deleting processed requests
  if ( $result == 1 )
  {
      unlink $work_on;
  }

  sleep(2);
}

Take special note to my "mild" attempt and DRYing up your code by using the find_by subroutine. You had a LOT of duplicate code in your original script, which I moved into a single sub routine. DRY eq 'Don't Repeat Yourself'.

Jim
  • 1,499
  • 1
  • 24
  • 43
  • 2
    Re "*your use of Proc::Daemon::Init is suspect.*", It's a correct and documented usage. That said, it would be more useful to instruct it to redirect STDERR to an actual file instead of the default `/dev/null`. – ikegami Jun 21 '17 at 04:44