in reply to Perl - Source code review

The project is not small. Do you really think someone will review it for free?

The person who wrote the code was probably more skilled in a different programming language (C?). There is no strict. I don't understand some of the idioms used there, e.g.:

while (defined(pop @LogFileList)) {};

Basically, we try to help people who want to learn. Describe what the project does, what you already tried and where you got stuck. Then we can help you.

لսႽ† ᥲᥒ⚪⟊Ⴙᘓᖇ Ꮅᘓᖇ⎱ Ⴙᥲ𝇋ƙᘓᖇ

Replies are listed 'Best First'.
Re^2: Perl - Source code review
by SuicideJunkie (Vicar) on Nov 13, 2014 at 14:38 UTC
    The project is not small. Do you really think someone will review it for free?

    I'm pretty sure the free reviewer is the student Bheemamahesh.

    As I understand the question, the project is not necessarily intended to accomplish anything... but it is supposed to do nothing or anything in a safe manner. And there is a bug hidden in the code to find.

    Based on that, I would suggest starting with all the standard advice (strict, warn, 3-arg open, etc, etc) and take close notice of the problems that they are intended to prevent. Make a big list (Variable typos, undefined values, barewords, borked references, changing the r/w type of a filehandle, etc.)

    With those kinds of problems in mind, run through the code in your head, and imagine/simulate what would happen on hostile input.

Re^2: Perl - Source code review
by Jenda (Abbot) on Nov 13, 2014 at 10:15 UTC

    I assume the curlies are not empty in the original, right? The loop removes the elements from the array one by one starting at the end and stops once there are no more elements to remove. The last element is at the start of the first iteration, before the body of the loop runs and as it is not assigned anywhere it most probably is not processed.

    If the curlies were empty and the array was not tie()d, then the line would be equivalent to @LogFileList = ();

    The only case in which it would make some sense to use that line with empty curlies would be if

    1. the array was tie()d
    2. removing elements from the tie()d array had some important side-effects
    3. you wanted the side effects to occure starting with the last element

    Jenda
    Enoch was right!
    Enjoy the last years of Rome.

      I assume the curlies are not empty in the original, right?
      Wrong :)
      If the curlies were empty and the array was not tie()d, then the line would be equivalent to @LogFileList = ();
      Except worse :)  perl -E 'my @ary = (0, 1, undef, 3); while (defined(pop @ary)) {}; say join "|", @ary' Which is probably not what they wanted.
      The only case in which it would make some sense to use that line with empty curlies would be if...
      4. the entire thing is just that bad.

        Ouch, you are right, I am wrong. It only removes the elements up to the first undef. Probably now what was intended either.

        Jenda
        Enoch was right!
        Enjoy the last years of Rome.

Re^2: Perl - Source code review
by Anonymous Monk on Nov 13, 2014 at 08:24 UTC

    The project is not small. Do you really think someone will review it for free?

    Hmm, how big is it?

    My anonymous proxy turned the zipfile into a bigger .exe ... I did not download :)