Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Agregar attributes a funciones printf-like #148

Merged
merged 2 commits into from
Nov 10, 2021
Merged

Conversation

RaniAgus
Copy link
Contributor

De la nada me encontré con que este atributo es el que hace que el compilador nos chille cuando la estemos pifiando con printf(...). Al mergear este PR, va a pasar lo mismo en cualquier función de log 🎉

#include <commons/log.h>

int main() {
    t_log* logger = log_create("yo-tengo-tu.log", "I_GOT_YOUR_LOG", 1, 0);
    log_info(logger, "%s");
    return 0;
}
$ gcc -g -o main main.c -lcommons
main.c: In function ‘main’:
main.c:5:21: warning: format ‘%s’ expects a matching ‘char *’ argument [-Wformat=]
    5 |     log_info(logger, "%s");
      |                    ~^
      |                     |
      |                     char *

@mgarciaisaia
Copy link
Member

¡Grosso finding!

Acá está la documentación oficial de GCC (en la sección format).

Me pregunto si agregándolo en la macro que define cada una de las funciones de log no quede más prolijo (por agregarlo una sola vez, y porque podemos agregar o borrar niveles nuevos de log y mantener el comportamiento).

Pero también me parece que está bueno tenerlo en la interfaz "pública" de la biblioteca - como está el PR actualmente.

Si no convergemos a una respuesta convincente en un par de días, mergeemos así como está 👍

@RaniAgus
Copy link
Contributor Author

RaniAgus commented Nov 10, 2021

Ahí revisé y la única forma que tiene el compilador de darse cuenta que tiene que hacer esa validación es a través de la interfaz pública. Debe ser porque, al hacer gcc, el changuito que preprocesa pega el código del header del #include para que luego el changuito que compila revise en base a ese prototipo, algo como:

#include <stdio.h>
#include <stdbool.h>
#include <sys/types.h>

typedef enum {
    LOG_LEVEL_TRACE,
    LOG_LEVEL_DEBUG,
    LOG_LEVEL_INFO,
    LOG_LEVEL_WARNING,
    LOG_LEVEL_ERROR
} t_log_level;

typedef struct {
    FILE* file;
    bool is_active_console;
    t_log_level detail;
    char *program_name;
    pid_t pid;
} t_log;

/**
* @NAME: log_create
* @DESC: ...
*/
t_log* log_create(char* file, char *program_name, bool is_active_console, t_log_level level);

/**
* @NAME: log_info
* @DESC: ...
*/
void log_info(t_log* logger, const char* message, ...)  __attribute__((format(printf, 2, 3)));

int main() {
    t_log* logger = log_create("yo-tengo-tu.log", "I_GOT_YOUR_LOG", 1, 0);
    log_info(logger, "%s");
    return 0;
}

De hecho, se puede compilar este archivo con el flag -lcommons y obtener el mismo resultado que en el ejemplo anterior.

@RaniAgus RaniAgus changed the title Logs: agrego attributes a funciones printf-like Agregar attributes a funciones printf-like Nov 10, 2021
@RaniAgus
Copy link
Contributor Author

Me había olvidado que commons/string.h también tiene un par de funciones para dar formato tipo printf, ahí se las agregué:

#include <commons/string.h>

int main() {
    char* s = string_from_format("Defensa y Justicia tiene %d libertadores");
    string_append_with_format(&s, " y %d descensos");

    return 0;
}
$ gcc -g -o main main.c -lcommons
main.c: In function ‘main’:
main.c:4:61: warning: format ‘%d’ expects a matching ‘int’ argument [-Wformat=]
    4 |     char* s = string_from_format("Defensa y Justicia tiene %d libertadores");
      |                                                            ~^
      |                                                             |
      |                                                             int
main.c:5:40: warning: format ‘%d’ expects a matching ‘int’ argument [-Wformat=]
    5 |     string_append_with_format(&s, " y %d descensos");
      |                                       ~^
      |                                        |
      |                                        int

Copy link
Member

@mgarciaisaia mgarciaisaia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

¡Grosso!

No sabía si gcc chequeaba en la declaración o en la definición.

Listo, mandale cumbia nomás 👍

@mgarciaisaia mgarciaisaia merged commit 526e520 into master Nov 10, 2021
@RaniAgus RaniAgus deleted the feat/attributes branch December 8, 2021 05:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants