visite sito

[submax82]

avete idea di che possibile bug ci sia in questo codice che uso per contare le visite del mio sito?

Codice: Seleziona tutto

<?php
session_start();
$database_txt = "visite.txt";
if( (file_exists("$database_txt")) == false )
{
  touch("$database_txt");
  chmod("$database_txt", 0664);
  $db_file = @fopen("$database_txt","w");
  @fwrite($db_file, 1);
  @fclose($db_file);
}
if(!$_SESSION["views"])
{
$tot = file("$database_txt");
$tot[0]++;
$db_file = @fopen("$database_txt", "w");
@fputs($db_file , $tot[0]);
@fclose($db_file);
$_SESSION["views"] = 1;
}
?>

funziona bene ma è già 2 volte che mi capita che riparte da 1... poi le riaggiusto io e funziona, anche se non ricordo mai il numero esatto... quindi metto sempre meno, è una cosa antipatica ma non capisco il problema, ammesso che ci sia

[SIV]

Non è che qualche volta per qualche x motivo, il file del database non viene trovato, la condizione diventa "false" ed il file viene ricreato ?

[raffaele181188]

Beh, tutto è possibile, però visto che la cosa si ripete
SubMax, ora non ho PHP sottomano per fare alcune prove, comunque può darsi anche che il problema sia quel $tot[0]++, visto che si tratta di una stringa e magari può contenere uno spazio o una newline. A parte questo, ci sono delle cose "strane" in quel codice

• $database_txt è sempre usato tra apici doppi... perchè?
• touch() serve per fare altro: per creare un file si usa fopen($nomeFile, 'w')
• $tot[0], come ti dicevo, è una stringa, quindi andrebbe come minimo dato a trim()

Comunque effettivamente è strano, appena posso faccio una prova

[raffaele181188]

UPDATE
Ho fatto un test e boh... funziona tutto in locale (ripetuto diecimila volte). Ho usato il tuo codice cambiando solo una riga in questo modo per adattarlo al test:

Codice: Seleziona tutto

if(!isset($_SESSION["views"]))


[submax82]

in php sono alle prime armi, sto imparando quindi... booo ... mi affido a voi

[conraid]

Ognuno ha i propri metodi e abitudini, però, a parte il discorso delle sessioni, io farei così

Codice: Seleziona tutto

$database_txt = 'visite.txt';
if(file_exists($database_txt)) {
   $tot = file_get_contents($database_txt);
} else {
   $tot = 1;
}
file_put_contents($database_txt , $tot);


non ha senso il codice di creazione del file, tanto se non esiste viene creato automaticamente

imho mai usare @, nemmeno in produzione

[raffaele181188]

Per risolvere il tuo errore non so che dirti, puoi provare a tenere un log separatamente perchè mi pare che nonostante tutto non ci siano errori

Offtopic: Ovviamente sul codice sono d'accordo con Conraid, solo che è sempre pericoloso dare consigli perchè non tutti la prendono bene
La @, invece, io la userei sempre: l'eventuale warning che viene fuori è troppo troppo dettagliato per mostrarlo al mondo (e poi può scombussolare l'output a livelli inimmaginabili). Quindi o la chiocciola oppure ci si accerta di tappare la bocca a PHP

[targzeta]

Anche secondo me l'errore è sull'incremento della variabile. Guarda questo piccolo codice inserito nel file casa.php:

Codice: Seleziona tutto

<?php
  $tot = file_get_contents('visite.txt');
  $tot++;

  if ( $tot == 1 )
      echo 'ops ';

  file_put_contents('visite.txt', $tot);
?>


Ora guarda cosa può succedere:

Codice: Seleziona tutto

$> echo 1 > visite.txt
$> php casa.php
ops

La variabile non è stata incrementata ed è rimasta 1, eppure:

Codice: Seleziona tutto

$> cat -e visite.txt
1$
$> file visite.txt
visite.txt: ASCII text

Mentre con questo codice:

Codice: Seleziona tutto

<?php
  $tot = trim(file_get_contents('visite.txt'));
  $tot++;

  if ( $tot == 1 )
      echo 'ops ';

  file_put_contents('visite.txt', $tot);
?>

Abbiamo un esecuzione corretta:

Codice: Seleziona tutto

$> echo 1 > visite.txt
$> php casa.php
$> cat -e visite.txt
2
$> file visite.txt
visite.txt: very short file (no magic)

Come vedi quando prelevi da un file una stringa che sai essere un numero è meglio ripulirla sempre da eventuali spazi o caratteri speciali.

Quindi io userei il codice di conraid aggiungendo una trim() e incrementando la variabile, conraid se ne è dimenticato .

Emanuele

[targzeta]

...imho mai usare @, nemmeno in produzione

Secondo me dipende. Se non controlli il valore di ritorno della funzione allora togliere la @ aiuta molto, però se controlli il valore di ritorno puoi usare la @ per tradurre l'errore in un qualcosa di più leggibile.

Emanuele

[aschenaz]

Io uso questo codice scritto diversi anni fa e va tuttora bene:

Codice: Seleziona tutto

function counter() {
      $counterfile=getcwd().DIRECTORY_SEPARATOR."var/cnt.log";
      if(!file_exists($counterfile)) {
         $cmake=fopen($counterfile,"w");
         chmod($counterfile,0777);
         fputs($cmake,"0");
         fclose($cmake);
      }
      if(!isset($_SESSION["counter"])) {
         $cfop=fopen($counterfile,"r+");
         $acc=fgets($cfop,10);
         $acc++;
         fseek($cfop,0);
         fputs($cfop,$acc);
         fclose($cfop);
         $_SESSION["counter"]=$acc;
      } else $acc=$_SESSION["counter"];
      return $acc;
   }
echo counter(); // dove serve


Uniche avvertenze: dare i permessi in scrittura alla directory (che io ho
chiamato var); avviare le sessioni, se non già fatto.

[conraid]

Quindi io userei il codice di conraid aggiungendo una trim() e incrementando la variabile, conraid se ne è dimenticato .

ehm... premetto che di programmazione capisco il giusto e qui siete tutti migliori di me, e soprattutto sbaglierò qualcosa, ma non me ne sono scordato. Quello era un codice di esempio, senza controllo errori (come dici tu vanno controllati, io odio @ perché tra sviluppo e produzione devi modificare tutto, invece se li gestisci puoi lavorare a livelli di una variabile iniziale, anche semplicemente mettendo di non mostrare errori) e senza tante cose, del resto trim lo uso sempre "pulire" le stringhe, ma...
se devo essere sincero in quel caso particolare farei così

Codice: Seleziona tutto

$database_txt = 'visite.txt';
if(file_exists($database_txt)) {
   (int)$tot = file_get_contents($database_txt)+1;
} else {
   $tot = 1;
}
file_put_contents($database_txt , $tot);

lo so che il casting in php è visto di cattivo occhio, ma così sai che è un intero
naturalmente manca sempre la gestione degli errori, in questo caso anche un gettype, non si sa mai cosa possa succedere

[raffaele181188]

Conraid, se il cast non piace c'è sempre intval() che fa esattamente la stessissima cosa ma è una funzione
Il discorso della @ per me va affrontato così: gli errori sono preziosissimi, quindi mettere a tacere una funzione in quel modo può essere addirittura controproducente perchè ti impedisce il debug. Però non si può mostrare a chiunque

Codice: Seleziona tutto

Php[E_WARNING] in /var/www/htdocs/sito1/path/to/file.php at line 13: Errore trallallà

perchè è proprio un pugno in faccia alla sicurezza
L'ideale è sempre agire a monte settando opportunamente il comportamento di PHP con gli errori. Tuttavia in uno scriptino del genere mi pare che la @ sia più che sufficiente

Offtopic: @aschenaz
La chiamata a getcwd() è rischiosa: se lo script viene incluso il risultato è imprevedibile. Meglio dirname(__FILE__)
Dopo tanti anni, potresti anche aggiornare lo script facendogli usare un db vero in modo da poter avere statistiche più utili a disposizione Per esempio, un contatore di questo tipo non è in grado di riferire nè quali documenti sono stati richiesti nè quando sono avvenute le richieste.. Tiene traccia solo del numero globale di utenti che hanno visitato il sito, ma è un'indicazione sostanzialmente inutile se non è riferita a qualche indice temporale

[submax82]

grazie a tutti, rileggo tutto bene dopo...

[submax82]

grazie ai vostri consigli ho modificato il codice ed ecco il risultato:

Codice: Seleziona tutto

<?php
session_start();
$database_txt = 'db_views.txt';
if(!$_SESSION["views"])
{
   if(file_exists($database_txt))
      $tot = intval(trim(file_get_contents($database_txt)));
   else
      $tot = 0;
   $tot++;
   //file_put_contents required PHP5
   $db_file = @fopen($database_txt, "w");
   @chmod($database_txt, 0664);
   @fseek($db_file, 0);
   @fputs($db_file, $tot);
   @fclose($db_file);
   $_SESSION["views"] = $tot;
}
?>

sembra funzionare bene... grazie a TUTTI

[submax82]

qualcuno si ricorda se il numero di visite ripristinato del mio sito è esatto? tanto per essere sicuri di non aver fatto cavolate