slacky.eu

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

<?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

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

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

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"]))

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

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

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

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

conraid ha scritto:...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

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.

spina ha scritto: 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

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

grazie a tutti, rileggo tutto bene dopo...

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

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

slacky.eu

visite sito

visite sito

Re: visite sito

Re: visite sito

Re: visite sito

Re: visite sito

Re: visite sito

Re: visite sito

Re: visite sito

Re: visite sito

Re: visite sito

Re: visite sito

Re: visite sito

Re: visite sito

Re: visite sito

Re: visite sito