Segmentation Fault en fpg_save()?

Started by gecko, October 06, 2017, 11:00:00 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

SplinterGU

#15
Quote from: daltomi on October 11, 2017, 05:13:42 AM
Bueno, ahora si voy a decir que esta bien y que esta mal :P
La función del fallo y original de bennu es esta:

int gr_save_fpg( int libid, const char * filename )


Y el problema esta aquí, en 32 y 16 (lo encontré por el parche de pixtudio)

       // /* Write the bitmap pixel data */

            for ( y = 0 ; y < gr->height ; y++ ) {
               
                switch( bpp ) {
                    case    32:
                            file_writeUint32A( fp, ( uint32_t * ) ( uint8_t * ) gr->data + gr->pitch * y, gr->width);
                            break;

                    case    16:
                            file_writeUint16A( fp, ( uint16_t * ) ( uint8_t * ) gr->data + gr->pitch * y, gr->width);
                            break;

                    case    8:
                    case    1:
                            file_write( fp, ( uint8_t * )gr->data + gr->pitch*y, gr->widthb );
                            break;
                }
            }


Básicamente, por lo que entiendo, file_write{16,32} no sabe cuando "detenerse".

Pixtudio, como parche, usa malloc(), memcpy() y free() para crear un nuevo segmento de memoria('block') de tamaño igual a gr->widthb y
luego copia gr->data alineado a ese segmento nuevo, ahora si, file_write{16,32} sabe cuando detenerse, asi se comporta igual en el case 8.

            /* Write the bitmap pixel data */

            if (bpp > 8) {
                if (!(block = malloc(gr->widthb))) {
                    file_close(fp);
                    return 0; /* No memory */
                }
            }

            for (y = 0; y < gr->height; y++) {
                if (bpp > 8) {
                    memcpy(block, (uint8_t *)gr->data + gr->pitch * y, gr->widthb);
                    if (bpp == 16) {
                        /*                        gr_convert16_ScreenTo565(( uint16_t * )block,
                         * gr->width ); */
                        file_writeUint16A(fp, (uint16_t *)block, gr->width);
                    } else
                        file_writeUint32A(fp, (uint32_t *)block, gr->width);
                } else {
                    file_write(fp, (uint8_t *)gr->data + gr->pitch * y, gr->widthb);
                }
            }

            if (bpp > 8)
                free(block);


Los problemas del parche de pixtudio:
1- malloc esta en el ámbito del for loop nmaps que itera unas ~32 veces * gr->widthb = ~232 bytes.
2- memcpy esta en el ámbito del for loop gr->height.

Mi solución es no usar malloc y usar memoria automática.


    /* Write each map */
   
    uint32_t dummy32; <--- Fuera de nmaps
    uint16_t dummy16;
   
    for ( n = 0 ; n < nmaps ; n++ ) {
    .......................................................................
    .......................................................................
   
        /* Write the bitmap pixel data */
       
        for ( y = 0 ; y < gr->height ; y++ ) {
               
                uint8_t *src = ( uint8_t * ) gr->data + gr->pitch * y;
                uint8_t *end = ( uint8_t * ) src + gr->widthb;

                switch( bpp ) {
                    case    32:
                            dummy32 = *( uint32_t * ) end;
                            *end = 0;
                            file_writeUint32A( fp, ( uint32_t * ) ( uint8_t * ) src, gr->width);
                            *end = ( uint32_t ) dummy32;
                            break;

                    case    16:
                            dummy16 = *( uint16_t * ) end;
                            *end = 0;
                            file_writeUint16A( fp, ( uint16_t * ) ( uint8_t * ) src, gr->width);
                            *end = ( uint16_t ) dummy16;
                            break;

                    case    8:
                    case    1:
                            file_write( fp, ( uint8_t * )gr->data + gr->pitch*y, gr->widthb );
                            break;
                }
            }
    .........................................................................

Básicamente, cuando file_write{32,16} recorre el bloque de memoria, se detiene en el terminador nulo 0, y para no perder el dato que ahí existía, lo guardo previamente.

Todo funciona bien con el archivo de test de gecko, con  set_mode(, , 32,) y set_mode( , , 16,).

Todo  dependerá que dicen los expertos si está aquí el bug o está en otro lado.






como que no sabe como detenerse ?!?!?!?!

el nul char no hace que se detenga.

se detiene por "gr->width", lo que has hecho con tus cambios es seguramente ocultar otro bug, con la nueva compilacion al reasignar las areas de memoria... eso pasa bastante cuando por ejemplo, compilamos algo en debug y en release... en release crashea y en debug no.

aun no tuve tiempo de ver esto... pero el poner un *end=0, no hace que se detenga el write... si fuera asi, se detendria con pixeles con valor 0.

esto no es una string que se guarda...

el problema es otro...
Download Lastest BennuGD Release: http://www.bennugd.org/node/2

daltomi

Ha ya, yo entendí cualquier cosa ;D

Gracias Splinter  ;)

SplinterGU

no pasa nada, cuando tenga la solucion aviso...
Download Lastest BennuGD Release: http://www.bennugd.org/node/2

SplinterGU

#18
bien... fixeado!

a mi no me reventaba el ejecutable, pero grababa mal el fpg... era una cuestion de casteo... me comi unos parentesis...


--- modules/mod_map/file_fpg.c  (revisión: 343)
+++ modules/mod_map/file_fpg.c  (copia de trabajo)
@@ -320,11 +320,11 @@
             for ( y = 0 ; y < gr->height ; y++ ) {
                 switch( bpp ) {
                     case    32:
-                            file_writeUint32A( fp, ( uint32_t * ) ( uint8_t * )gr->data + gr->pitch*y, gr->width );
+                            file_writeUint32A( fp, ( uint32_t * ) ( ( uint8_t * )gr->data + gr->pitch*y), gr->width );
                             break;

                     case    16:
-                            file_writeUint16A( fp, ( uint16_t * ) ( uint8_t * )gr->data + gr->pitch*y, gr->width );
+                            file_writeUint16A( fp, ( uint16_t * ) ( ( uint8_t * )gr->data + gr->pitch*y), gr->width );
                             break;

                     case    8:


ahi esta el diff

gracias por el reporte, el sample y los analisis...
Download Lastest BennuGD Release: http://www.bennugd.org/node/2