Discussion :

[Nekkro]

Il est temps que je donne un peu plus d'informations, d'autant que j'ai dû affiner mon problème (merci koala).

L'application qui doit utiliser le Logger est une application serveur, donc j'ai besoin de performances particulièrement rapides.
Mais en parallèle, j'ai besoin de pouvoir consulter les logs en même temps que le serveur tourne, afin de ne pas avoir besoin de l'éteindre ou quoi que ce soit de ce genre pour pouvoir consulter des informations critiques.

La particularité de mon Logger et de LogLevel, c'est que certains niveaux sont logués dans un fichier (et dossier) spécifique jour après jour, tandis que d'autres sont logués dans un unique fichier qui ne change pas.

En plus de ça, j'affiche des informations directement dans la console pour lorsque je debug en direct avec le client.

Cela veut dire que plusieurs niveaux peuvent avoir besoin d'un même fichier, ou pas.

Donc il va falloir non pas que j'intègre l'ofstream directement dans LogLevel, mais plutôt que j'y mette une référence.

@Ekleog > J'avais fait en sorte d'afficher quelque chose dans le destructeur de LogLevel, et à l'exécution je voyais bien que lorsque j'exécutais :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

logger.register_level ("truc", LogLevel(/* ... */));

Le destructeur était appelé, et par conséquent quand l'objet dans ma map était détruit à la sortie du scope, j'avais un crash car deux destructions d'ofstream sur le même fichier.

Pour ce qui est des unique_ptr, je suis presque sûr de mal m'y prendre puisque ça ne marche pas mais que mon <bits/unique_ptr.h> contient bien :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
// Assignment.
      unique_ptr&
      operator=(unique_ptr&& __u) noexcept
      {
	reset(__u.release());
	get_deleter() = std::forward<deleter_type>(__u.get_deleter());
	return *this;
      }
 
      template<typename _Up, typename _Ep, typename = typename
	std::enable_if
	  <std::is_convertible<typename unique_ptr<_Up, _Ep>::pointer,
			       pointer>::value
	   && !std::is_array<_Up>::value>::type>
	unique_ptr&
	operator=(unique_ptr<_Up, _Ep>&& __u) noexcept
	{
	  reset(__u.release());
	  get_deleter() = std::forward<_Ep>(__u.get_deleter());
	  return *this;
	}

J'ai essayé avec une petit structure ne contenant qu'un seul unique_ptr de le rentrer dans une map, mais je ne sais pas quel prototype donner à ma fonction pour que le move soit appelé (à chaque fois il me dit que j'ai pas de constructeur/operateur de copie parce que sinon il serait malformé ...).

[Ekleog]

koala01: OMG, je ne me souvenais plus que windows était aussi nul que ça ...
Sinon, c'est vrai qu'un pimpl pourrait servir, mais pour le cas je pense qu'un flag de compilation suffirait -- pourquoi faire un indirection inutile, étant donné qu'après la compilation le comportement est fixé ?

Nekkro:

Si ton serveur tourne sous un linux, tout va bien avec le concept de garder le fichier ouvert.
Si il toune sous windows, et que tu as beaucoup d'informations à logger, ça va être plus dur.
Mais qui a un serveur sous windows ? (/me troll inside)

Sinon, tu dis que pour utiliser plusieurs LogLevel avec le même fichier, il faudra mettre une référence, mais c'est plutôt un shared_ptr qu'il te faudrait ici.

Sinon, logger.register_level ("truc", LogLevel(/* ... */)); va clairement déclencher le destructeur de LogLevel, mais pas à cause de la référence constante : c'est en fait à cause du LogLevel temporaire que tu créées, qui sera forcément détruit à la fin de l'instruction.
Que voudrais-tu que ce code fasse d'autre ?

Pour ce qui est des unique_ptr, je suis presque sûr de mal m'y prendre puisque ça ne marche pas mais que mon <bits/unique_ptr.h> contient bien :

De toute façon, si tu arrives à instancier std::unique_ptr<un_type> ; c'est forcément qu'il a un move-ctor. Sinon, unique_ptr ne servirait à rien.
Quand utilises-tu unique_ptr ? Penses-tu bien à utiliser std::move(ptr) ?

Sinon, une struct de base rentrable dans une map (non testé) :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
struct S {
   std::unique_ptr<int> p;
   S(std::unique_ptr<int> &&pp) : p(std::move(pp)) {} // Ctor a partir d'unique_ptr
   S(S &&o) : p(std::move(o.p)) {} // Move-ctor
   S & operator = (S && o) {
      p = std::move(o.p);
      return *this;
   }
};
 
// ...
std::map<int, S> m;
m.emplace(1, new int(42)); // Plus certain de la syntaxe d'emplace, a verifier
m[2] = S(new int(1337));
// Ou (pas sur) :
m[2] = std::move(S(new int(1337)));

[JolyLoic]

koala01: OMG, je ne me souvenais plus que windows était aussi nul que ça ...

Pas nul, différent.

Personnellement, étant plus habitué à windows, c'est le comportement de linux que je trouve rétrograde (plein d'opérations sans API qui obligent à exécuter des lignes de commandes et à parser les résultats, où à parser un fichier de config pour le modifier, pas de gestion de type comptage de référence pour les objets système (sémaphoe, mutex...)). Mais ce sont avant tout des choix de design et de philosophie.

Et puis, sous windows, il est possible d'avoir un fichier ouvert par deux process en même temps (http://msdn.microsoft.com/en-us/libr...=vs.85%29.aspx, en particulier le paramètre sharemode), ce n'est juste pas le comportement par défaut (ce que je respecte, car dans le cas général, deux process qui accèdent au même fichier, c'est une corruption qui ne demande qu'à apparaître). Et donc ce n'est pas un comportement facilement utilisable avec les flux C++.

[koala01]

koala01: OMG, je ne me souvenais plus que windows était aussi nul que ça ...
Sinon, c'est vrai qu'un pimpl pourrait servir, mais pour le cas je pense qu'un flag de compilation suffirait -- pourquoi faire un indirection inutile, étant donné qu'après la compilation le comportement est fixé ?

En fait, Nekkro m'a donné raison avec ses explications complémentaires (bon, d'accord, ce n'est qu'un hasard )

En effet, en lisant ce qu'il doit faire, je me dis qu'il serait quand meme préférable qu'un Logger ne manipule qu'un seul fichier, et que l'on puisse en profiter pour gérer l'écriture (ou non) en fonction de la criticité...

Bon, on s'écarte un peu du pimpl, mais, au final, on aurait quelque chose qui serait proche de

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
enum LogLevel
{
    ERROR_Critical, // ce qui provoque l'arret du service
    /* ... quelque niveau d'erreur non critiques */
    WARNING, // un avertissement
    /* éventuellement des niveau d'avertissement */
    INFO // juste une information
}
class Logger
{
    public:
    static Logger &instance()
    {
         static Loger inst;
         return inst;
    }
    class Logger_impl
    {
        public:
            Logger_impl(std::string const & filename):filename_(filename){}
            virtual ~Logger_impl(){}
            virtual void write(std::string const &, LogLevel ) = 0;
    };
    void addLogger(Logger_impl * )
    {
        loggers_.push_back(logger);
    }
    void  write(std::string const & message, LogLevel level )
    {
 
        BOOST_FOREACH(Logger_impl * logger, loggers_)
       {
           logger->write(message, level);
       }
    }
    private:
    std::vector<Logger_impl *> loggers_;
 
    Logger(){}
    ~Logger()
    {
        BOOST_FOREACH(Logger_impl * logger, loggers_)
       {
           delete logger;
       }
    }
};
/* plusieurs classes héritant de Logger_impl pour les différentes 
 * politiques (en terme de gestion du fichier et en terme d'écriture
 * selon la criticité)
 */
 
void log(std::string const & message, ErrorLevel level)
{
     Logger::instance().write(message, level);
}

Je ne suis pas, par nature, très favorable à l'utilisation des singletons, mais, en l'occurrence, je me dis que, dans ce cas particulier, son utilisation peut se justifier

On aurait, au début de l'application quelque chose comme

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
int init()
{
 
    Logger::instance().addLogger(new LoggerErrorOnly("Path/To/error.log");
    Logger::instance().addLogger(new LoggerUsage("Path/To/use.log");
    /* pourquoi pas */
 
    Logger::instance().addLogger(new OutputToConsole();
    /* ... */
}

et, au niveau de l'utilisation de simples

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
log("le message de la mort qui tue", ERROR_Critical);
/* ou */
log("un autre message", WARNING);

En prenant soin de séparer la classe Logger de la fonction log dans les fichiers d'en-tete (et surtout en implémentant les fonctions dans des fichiers d'implémentation et non inline comme je l'ai fait ici ), l'utilisation du Logger peut se faire de manière totalement transparente pour tout codeur qui ne va pas jeter un oeil à la fonction main

De plus, comme on travaillerait sur avec des pointeurs sur loggers_impl (et que l'on a clairement déterminé qui s'occupe de les détruire ), il n'y aurait meme plus le problème d'une éventuelle tentative de copie du fichier, si celui-ci est ouvert au niveau des classes héritant de Logger_impl

[Nekkro]

Merci à tous pour vos conseils et critiques, j'ai finalement résolu mon problème en créant utilisant les unique_ptr<ofstream> que je créais dans Logger plutôt que dans LogLevel.

Voici un petit sample final

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
Logger logger;
logger.register_level (LogLevel (LogPolicy::FILE_POLICY, "./logs/debug.log", Log::DEBUG));
logger.target (Log::DEBUG) << "Un message de test.\n";
logger.register_level (LogLevel (LogPolicy::FOLDER_POLICY, "./logs/debug/", Log::ALERT));
logger.target (Log::ALERT) << "Un message de test.\n";