in reply to Writing general code: real world example - and doubts!

Untainting variables that you've already checked to be good? That's the battle you should be fighting because it's a recipe for disaster. Don't wuss out: if you want taint checking, don't subvert it by giving programmers a way to ignore it. :)

The trick to any of these sorts of decisions is to look at the rest of the code and figure out which style it uses, then try to use the same model. If most of the interface changes arguments in place, do that. If most of the interface leaves arguments alone and returns the munged values, do that. Beyond that, figure out which one makes the problem easier to solve and the code easier to read ( the optimal combination of those!), and do that.

Once you figure out what you want, clearly document it.

But, if you're spending all your time worrying about little details like this, you're probably avoiding much bigger problems that you don't want to think about. Put something in place and move on. :)

--
brian d foy <brian@stonehenge.com>
  • Comment on Re: Writing general code: real world example - and doubts!

Replies are listed 'Best First'.
Re^2: Writing general code: real world example - and doubts!
by Zaxo (Archbishop) on Apr 05, 2005 at 02:57 UTC

    ++ your observation on not giving an out on detainting.

    I've got a solution to detainting which I think is very well suited to refactoring old code. It can be retrofitted as a single chunk of initialisation code, and old code will automatically validate and detaint the rigged variable on every assignment or modification. 

    use Tie::Constrained qw/detaint/;
tie my $var, 'Tie::Constrained', sub {
    my $re = qr/whatever/;
    $_[0] =~ /$re/ and &detaint;
};

    [download]
    Later, when you say, $var = $tainted_thing; $var is validated and untainted, while $tainted_thing remains tainted and unmodified. The trick is that the &detaint call acts on an anonymous copy of $tainted_thing's value just before assignment to $var. (Tie::Constrained::detaint() is almost identical to frodo72's original detaint()).

    This does not address OP's excellent question about generalizing his detaint routine. I question the value of that as a general practice, but it may be useful for an application which must iterate over a bunch of similar-type values.

    After Compline,
    Zaxo

[reply]
[d/l]
[select]
      Meditation are good for meditating - so this thread is teaching me something about refactoring (which is theoretical), and something about tainting (which is practical). This is good for me.

      I like very much your solution. In particular, it makes me reflect that an in-place detainting could be dangerous, because I could end up not knowing whether a variable has already been detainted or not. Your approach seems to divide variables into two groups: tainted and not, and this seems reasonable.

      I only wonder how readability could be affected by this approach. The detainting method being under the surface, I fear that the naive programmer (er.. me) could be disorientated by seeing such an assignment and seeing $var used in potentially dieing places.

      Flavio (perl -e "print(scalar(reverse('ti.xittelop@oivalf')))")

      Don't fool yourself.
[reply]
[d/l]
[select]

        You're right, readability suffers with that approach. Because code is invoked at a distance, and there is no local indication of which variables are covered by the validation mechanism, the reader needs to absorb the ties before getting to the real code. That's likely to be missed by a maintainer.

        Documentation helps, comments help, but those may not be enough. You spotted the principal weakness.

        I'm not yet encouraging anyone to use Tie::Constrained in production. The API is not yet stable, simple as it is. I'll make a version 1.00 release when I think it's ready. If you have suggestions, I'd welcome them.

        After Compline,
        Zaxo

[reply]
Re^2: Writing general code: real world example - and doubts!
by polettix (Vicar) on Apr 05, 2005 at 08:45 UTC
    Well - quite a bad example to show my doubts, I should have sticked to the more "quiet" left_pad in the first time :)

    This demonstrates that there's always something to learn, particularly where you don't suspect it. I wrote the detaint function to "free" some directory and file names after verifying they're real dir and files with "-X" tests, so I was confident they were right and I felt comfortable about them. And I erroneously concentrated on the wrong problem - or better, I had nothing better to do than "Meditate" at that moment.

    But what I'll keep in mind is that whenever I strive for generality and reusability letting myself lose time, things could (and will hopefully be) used in contexts quite different from the ones I've developed them. So wondering how to make a detaint function the most general one is very similar to a Bad Thing, because encourages potentially bad coding practices (like not verifying your data).

    Once you figure out what you want, clearly document it.
    I hope this will become natural for me; I really admire the fact that this thing has become such a habit for a recognised Guru to forget to say "before you code", which I read nonetheless. I'm a naive programmer, and I tend to document things after I've done them, but I understand that writing documentation first will help me fixating ideas and stick to one solution instead of refactoring brainlessly.

    Just to answer myself a bit regarding the in-place modification avoiding copying large chunks of data, I remembered an old motto: A premature optimisation is a Bad Thing. For what old can mean in CS :)

    Flavio (perl -e "print(scalar(reverse('ti.xittelop@oivalf')))")

    Don't fool yourself.
[reply]
[d/l]

In Section Meditations