Forum: PC-Programmierung Debug ANSI C Programm, RaspberryPI.


von Welle 🧐 S. (w3llschmidt)


Lesenswert?

Hallo Gemeinschaft! Ein gesundes neues Jahr wünsche ich ...

Ich habe ein ganz blödes Problem.

Ein ANSIC Linux-Deamon (RaspberryPI), der via SYSFS (I2C, DS2482)
DS1820-Temperatur-Sensoren ausliesst und per HTTP_POST an eine
Middleware schickt.

Läuft ein paar Stunden 1A! und bleibt dann ohne auch nur das
kleinste Anzeichen stehen :-(

https://raw.github.com/w3llschmidt/1wirevz/master/1wirevz.c

Der Kern ist die while(1)-Schleife in main() + ds1820read() + 
http_post().

Keine spur in den Logs (syslog, messages, appache2) ...

Die Sysfs-Devices sind verfügbar und antworten ...

cat /sys/bus/w1/devices/28-0000042b410e/w1_slave
e1 01 4b 46 7f ff 0f 10 90 : crc=90 YES
e1 01 4b 46 7f ff 0f 10 90 t=30062

---

Gibt es eine Möglichkeit den Code in einem Debugger laufen zu lassen,
um zu sehen wo der stehen bleibt?

Wie gesgat, es läuft ganz sauber Stunden durch!

Grüsse, Henrik!

von Rufus Τ. F. (rufus) Benutzerseite


Lesenswert?

Nicht, daß ich jemals irgendwas mit einem Pi selbst gemacht hätte, aber 
sind GDB und DDD nicht auch dafür verfügbar?

von Welle 🧐 S. (w3llschmidt)


Lesenswert?

Hi Rufus ...

ja, GDB ist mit drauf ...

Reicht es im ersten Schritt mit 'gcc -g' zu compilieren und dann den
Deamon mit 'gdb deamon' + 'run' laufen zu lassen?

Schmeisst der GDB dann was raus? Oder muss ich von Hand breakpoints 
setzen?

von Andreas B. (andreas_b77)


Lesenswert?

Einfach ohne Breakpoints laufen lassen.

GDB meldet nur den Grund, wenn das Programm angehalten wird (etwa durch 
Breakpoint oder segmentation fault), ansonsten läuft das Programm 
einfach. Du kannst das Programm aber manuell in GDB mit Ctrl+C 
unterbrechen nachdem es nicht mehr reagiert und dann nachsehen, ob du 
den Grund entdeckst.

von Welle 🧐 S. (w3llschmidt)


Lesenswert?

Andreas B. schrieb:
> einfach. Du kannst das Programm aber manuell in GDB mit Ctrl+C
> unterbrechen nachdem es nicht mehr reagiert und dann nachsehen, ob du
> den Grund entdeckst.

Alles klar! Danke!

Mal sehen was es ist ... ist echt nervig :-(

von Andreas B. (andreas_b77)


Lesenswert?

Außerdem gibt es auch die Möglichkeit, einen GDB nachträglich an einen 
Prozess zu koppeln, falls man das Programm nicht aus GDB heraus 
gestartet hat. Im GDB statt "run" das Kommando "attach PID" verwenden, 
wobei PID für die Prozessnummer steht.

von Rolf Magnus (Gast)


Lesenswert?

Andreas B. schrieb:
> Außerdem gibt es auch die Möglichkeit, einen GDB nachträglich an einen
> Prozess zu koppeln, falls man das Programm nicht aus GDB heraus
> gestartet hat. Im GDB statt "run" das Kommando "attach PID" verwenden,
> wobei PID für die Prozessnummer steht.

Oder gleich beim Starten von GDB: "gdb EXECUTABLE PID" (mit den 
offensichtlichen Ersetzungen).

von Welle 🧐 S. (w3llschmidt)


Lesenswert?

Leider kein Erfolg ... gdb schweigt, syslog schweigt, der Schöpfer 
schweigt.

Das Programm bleibt einfach stehen :-(

Gibts irgendeine Möglichkeit zu 'sehen' in welcher Codezeile es hängt?

Henrik~

von 900ss (900ss)


Lesenswert?

Code instrumentieren und ein paar LEDs über Io schalten vielleicht? 
Damit bekommst du vielleicht raus an welcher Stelle es klemmt.

von Lukas K. (carrotindustries)


Lesenswert?

mal strace drauf losgelassen? Das loggt dir alle syscalls, die dein 
Programm tätigt.

von Sven B. (scummos)


Lesenswert?

Wenn das Programm stehen bleibt, in gdb Ctrl+C und dann "backtrace". Das 
sagt dir, in welcher Zeile es steht.

von thomil (Gast)


Lesenswert?

Keine Ahnung warum das Programm hängen bleibt, aber einen Fehler hab ich 
beim drüber sehen gefunden:
1
    char sensorid[3][64][17];
2
3
    /* später in main: */
4
    for (i=1; i<=3; i++) {
5
        /* out-of-bounds if i == 3 */
6
        sensorid[i][count][strlen(sensorid[i][count])-1] = '\0';
7
    }

Außerdem ist mir der Sinn der Zeile
1
sensorid[i][count][strlen(sensorid[i][count])-1] = '\0';

generell nicht klar. Warum schneidest du hier das letzte Zeichen im 
String ab?

von thomil (Gast)


Lesenswert?

Selbiges gilt natürlich auch für die 'vzuuid' Variable ein paar Zeilen 
weiter unten.

von thomil (Gast)


Lesenswert?

Und zusätzlich solltest du dringend mal die Warnungen die dir der 
Compiler gibt ansehen, und am Besten auch noch zusätzliche Warnungen 
aktivieren. Noch besserer wäre, damit man nicht in Versuchung kommt, 
Warnungen als Fehler zu betrachten.

Einfach mal das Programm mit:

gcc -Wall -Wextra -Werror <andere-parameter-wie-zuvor>

compilieren.

Dann fallen einem auch solche Sachen auf, wie dass die Funktion "double 
ds1820read(...)" unter Umständen gar nichts zurück gibt.

von Welle 🧐 S. (w3llschmidt)


Lesenswert?

Sven B. schrieb:
> Wenn das Programm stehen bleibt, in gdb Ctrl+C und dann "backtrace". Das
> sagt dir, in welcher Zeile es steht.

Alles klar, teste ich, danke!

von Welle 🧐 S. (w3llschmidt)


Lesenswert?

thomil schrieb:
> Keine Ahnung warum das Programm hängen bleibt, aber einen Fehler hab ich
> beim drüber sehen gefunden:
>
>
1
>     char sensorid[3][64][17];
2
> 
3
>     /* später in main: */
4
>     for (i=1; i<=3; i++) {
5
>         /* out-of-bounds if i == 3 */
6
>         sensorid[i][count][strlen(sensorid[i][count])-1] = '\0';
7
>     }
8
>

Mkay, 'i' ist in dem Fall die Anzahl der 1W-Master (3 Stück)

/sys/bus/w1/devices/w1_bus_master1/w1_master_slaves
/sys/bus/w1/devices/w1_bus_master2/w1_master_slaves
/sys/bus/w1/devices/w1_bus_master3/w1_master_slaves

Die sind fest verlötet, das sollte sich nie ändern ?!

> Außerdem ist mir der Sinn der Zeile
>
>
1
> sensorid[i][count][strlen(sensorid[i][count])-1] = '\0';
2
>
>
> generell nicht klar. Warum schneidest du hier das letzte Zeichen im
> String ab?

Damit entferne ich '#012' ... CR/LF ist das glaube ...

von Welle 🧐 S. (w3llschmidt)


Lesenswert?

thomil schrieb:
> Und zusätzlich solltest du dringend mal die Warnungen die dir der
> Compiler gibt ansehen, und am Besten auch noch zusätzliche Warnungen
> aktivieren. Noch besserer wäre, damit man nicht in Versuchung kommt,
> Warnungen als Fehler zu betrachten.
>
> Einfach mal das Programm mit:
>
> gcc -Wall -Wextra -Werror <andere-parameter-wie-zuvor>
>

Hab ich! Kommt nichts zurück ...

>
> Dann fallen einem auch solche Sachen auf, wie dass die Funktion "double
> ds1820read(...)" unter Umständen gar nichts zurück gibt.

Was meinstn konkret?

von Welle 🧐 S. (w3llschmidt)


Lesenswert?

Lukas K. schrieb:
> mal strace drauf losgelassen? Das loggt dir alle syscalls, die dein
> Programm tätigt.

Läuft schon seit ner Stunde :-)

Hatte ich paralell gefunden! Tolles Tool!

Danke für den Tip!

von Sven B. (scummos)


Lesenswert?

Kompilier' einfach mal mit -Wall -Werror, dann siehst du es. ;)
Damit dürfte sich das gar nicht übersetzen lassen.

von thomil (Gast)


Lesenswert?

Henrik Wellschmidt schrieb:
> Mkay, 'i' ist in dem Fall die Anzahl der 1W-Master (3 Stück)

Ja ich versteh schon wofür das 'i' gut ist. Das Problem ist, dass du das 
Array so definiert hast:

char sensorid[3][64][17];

Welche indices sind jetzt für sensorid gültig? Richtig, 0, 1, und 2. Du 
greifst aber mit 1, 2 und 3 darauf zu. Der index 3 ist außerhalb des 
Speicherbereichs den du für sensorid reserviert hast.
Bei 'vzuuid' selbes Spiel.

>> Dann fallen einem auch solche Sachen auf, wie dass die Funktion "double
>> ds1820read(...)" unter Umständen gar nichts zurück gibt.

> Was meinstn konkret?

Sieh dir einfach mal die Funktion an. Es ist möglich dass die Funktion 
das Ende erreicht, ohne jemals ein "return" ausgeführt zu haben.


Dass der gcc bei dir keine Warnungen ausgibt wenn du -Wall -Wextra 
verwendest kann ich nicht glauben. Bei mir gibt er alleine ohne diese 
Optionen schon ein paar Warnungen aus.

von Welle 🧐 S. (w3llschmidt)


Lesenswert?

Sven B. schrieb:
> Kompilier' einfach mal mit -Wall -Werror, dann siehst du es. ;)
> Damit dürfte sich das gar nicht übersetzen lassen.

Ich schwöre ... kein Mux!
1
pi@rpi ~/1wirevz $ sudo gcc -Wall -Wextra -Werror -o /usr/sbin/1wirevz 1wirevz.c -lconfig -lcurl
2
pi@rpi ~/1wirevz $

;-)

P.S. ich hatte noch nicht alle Änderungen gepusht, das Repo ist jetzt
aktuell ...

von Welle 🧐 S. (w3llschmidt)


Lesenswert?

thomil schrieb:
> char sensorid[3][64][17];
>
> Welche indices sind jetzt für sensorid gültig? Richtig, 0, 1, und 2. Du
> greifst aber mit 1, 2 und 3 darauf zu. Der index 3 ist außerhalb des
> Speicherbereichs den du für sensorid reserviert hast.
> Bei 'vzuuid' selbes Spiel.

Jetzt ja! Danke! Das is blöd ... muss ich ändern ...

von thomil (Gast)


Lesenswert?

Henrik Wellschmidt schrieb:
> Ich schwöre ... kein Mux!
> pi@rpi ~/1wirevz $ sudo gcc -Wall -Wextra -Werror -o /usr/sbin/1wirevz 1wirevz.c 
-lconfig -lcurl
> pi@rpi ~/1wirevz $

Tatsächlich. Auf der pi bekomme ich mit dem aktuellsten Code mit 
gcc-4.6.3 mit -Wall -Wextra keine Warnung.

Unter Ubuntu auf x86_64, ebenfalls mit gcc-4.6.3 findet er noch eine 
-Wformat Warnung.
Was mir aber gerade beim tippen auffällt ist, dass diese Warnungen 
wahrscheinlich durch unterschiedliche libconfig versionen zustande 
kommen.

Wenn ich aber am PC optimierungen aktiviere( -Wall -Wextra -O2 ) bekomme 
ich eine ganze Seite voller Warnungen. Auf der pi noch immer keine 
Einzige. Auch nicht wenn ich sie explizit aktiviere.

Die Warnings am PC beziehen sich auf ignorierte Rückgabewerte von fgets, 
chdir und write.

Immerhin weis ich jetzt, dass ich in Zukunft den raspian gcc bei 
Warnungen nicht trauen kann.

von Sven B. (scummos)


Lesenswert?

Henrik Wellschmidt schrieb:
> Sven B. schrieb:
>> Kompilier' einfach mal mit -Wall -Werror, dann siehst du es. ;)
>> Damit dürfte sich das gar nicht übersetzen lassen.
>
> Ich schwöre ... kein Mux!
Hihi, ja, mein Fehler. Ich hab das return in der ...read()-Funktion 
übersehen, weil Du das so komisch eingerückt hast (das gehört eins 
weiter nach rechts! ;)
Ich schätze mal, meinem Vorrender ging es da genauso.

von Welle 🧐 S. (w3llschmidt)


Lesenswert?

So, strace bringt mich etwas weiter ...
1
06:45:26.674305 socket(PF_NETLINK, SOCK_RAW, 0) = -1 EMFILE (Too many open files)
2
06:45:26.675968 gettimeofday({1357364726, 676693}, NULL) = 0
3
06:45:26.677572 setitimer(ITIMER_REAL, {it_interval={0, 0}, it_value={0, 0}}, {it_interval={0, 0}, it_value={298, 993473}}) = 0
4
06:45:26.679447 rt_sigaction(SIGALRM, {SIG_DFL, [], 0x4000000 /* SA_??? */}, NULL, 8) = 0
5
06:45:26.681162 clock_gettime(CLOCK_MONOTONIC, {108748, 703355349}) = 0
6
06:45:26.682655 clock_gettime(CLOCK_MONOTONIC, {108748, 704848286}) = 0
7
06:45:26.684120 socket(PF_INET, SOCK_STREAM, IPPROTO_TCP) = -1 EMFILE (Too many open files)
8
06:45:26.685812 clock_gettime(CLOCK_MONOTONIC, {108748, 708019152}) = 0
9
06:45:26.687340 gettimeofday({1357364726, 687951}, NULL) = 0
10
06:45:26.689125 gettimeofday({1357364726, 689805}, NULL) = 0
11
06:45:26.690810 writev(2, [{"1wirevz: HTTP_POST(): Couldn't c"..., 48}, {"\n", 1}], 2) = -1 EBADF (Bad file descriptor)
12
06:45:26.692599 send(1, "<14>Jan  5 06:45:26 1wirevz: HTT"..., 68, MSG_NOSIGNAL) = -1 ENOTSOCK (Socket operation on non-socket)
13
06:45:26.694275 close(1)                = 0
14
06:45:26.695665 socket(PF_FILE, SOCK_DGRAM|SOCK_CLOEXEC, 0) = 1
15
06:45:26.697152 connect(1, {sa_family=AF_FILE, path="/dev/log"}, 110) = 0
16
06:45:26.699046 send(1, "<14>Jan  5 06:45:26 1wirevz: HTT"..., 68, MSG_NOSIGNAL) = 68
17
06:45:26.701966 clock_gettime(CLOCK_MONOTONIC, {108748, 724182470}) = 0
18
06:45:26.703532 close(1023)             = 0
19
06:45:26.708300 read(1,

von Lukas K. (carrotindustries)


Lesenswert?

Scheint so, als würde sich syslog verschlucken, bau den doch mal aus.
So spontan fällt mir noch auf -1 EMFILE (Too many open files).
Was sagt ulimit -a dazu?

von Welle 🧐 S. (w3llschmidt)


Lesenswert?

Lukas K. schrieb:
> Scheint so, als würde sich syslog verschlucken, bau den doch mal aus.
> So spontan fällt mir noch auf -1 EMFILE (Too many open files).
> Was sagt ulimit -a dazu?

Hi Lukas, nee ick globe is nicht der syslog :-)

1
lsof -P -n -p 8467|wc -l
2
1057
3
4
(...)
5
6
1wirevz 8467   pi  999r   REG       0,14     4096  4002 /sys/devices/w1_bus_master3/28-0000042b410e/w1_slave
7
1wirevz 8467   pi 1000r   REG       0,14     4096  3992 /sys/devices/w1_bus_master1/28-0000042b89e6/w1_slave
8
1wirevz 8467   pi 1001r   REG       0,14     4096  4002 /sys/devices/w1_bus_master3/28-0000042b410e/w1_slave
9
1wirevz 8467   pi 1002r   REG       0,14     4096  3992 /sys/devices/w1_bus_master1/28-0000042b89e6/w1_slave
10
1wirevz 8467   pi 1003r   REG       0,14     4096  4002 /sys/devices/w1_bus_master3/28-0000042b410e/w1_slave
11
1wirevz 8467   pi 1004r   REG       0,14     4096  3992 /sys/devices/w1_bus_master1/28-0000042b89e6/w1_slave
12
1wirevz 8467   pi 1005r   REG       0,14     4096  4002 /sys/devices/w1_bus_master3/28-0000042b410e/w1_slave
13
1wirevz 8467   pi 1006r   REG       0,14     4096  3992 /sys/devices/w1_bus_master1/28-0000042b89e6/w1_slave
14
1wirevz 8467   pi 1007r   REG       0,14     4096  4002 /sys/devices/w1_bus_master3/28-0000042b410e/w1_slave
15
1wirevz 8467   pi 1008r   REG       0,14     4096  3992 /sys/devices/w1_bus_master1/28-0000042b89e6/w1_slave
16
1wirevz 8467   pi 1009r   REG       0,14     4096  4002 /sys/devices/w1_bus_master3/28-0000042b410e/w1_slave
17
1wirevz 8467   pi 1010r   REG       0,14     4096  3992 /sys/devices/w1_bus_master1/28-0000042b89e6/w1_slave
18
1wirevz 8467   pi 1011r   REG       0,14     4096  4002 /sys/devices/w1_bus_master3/28-0000042b410e/w1_slave
19
1wirevz 8467   pi 1012r   REG       0,14     4096  3992 /sys/devices/w1_bus_master1/28-0000042b89e6/w1_slave
20
1wirevz 8467   pi 1013r   REG       0,14     4096  4002 /sys/devices/w1_bus_master3/28-0000042b410e/w1_slave
21
1wirevz 8467   pi 1014r   REG       0,14     4096  3992 /sys/devices/w1_bus_master1/28-0000042b89e6/w1_slave
22
1wirevz 8467   pi 1015r   REG       0,14     4096  4002 /sys/devices/w1_bus_master3/28-0000042b410e/w1_slave
23
1wirevz 8467   pi 1016r   REG       0,14     4096  3992 /sys/devices/w1_bus_master1/28-0000042b89e6/w1_slave
24
1wirevz 8467   pi 1017r   REG       0,14     4096  4002 /sys/devices/w1_bus_master3/28-0000042b410e/w1_slave
25
1wirevz 8467   pi 1018r   REG       0,14     4096  3992 /sys/devices/w1_bus_master1/28-0000042b89e6/w1_slave
26
1wirevz 8467   pi 1019r   REG       0,14     4096  4002 /sys/devices/w1_bus_master3/28-0000042b410e/w1_slave
27
1wirevz 8467   pi 1020r   REG       0,14     4096  3992 /sys/devices/w1_bus_master1/28-0000042b89e6/w1_slave
28
1wirevz 8467   pi 1021r   REG       0,14     4096  4002 /sys/devices/w1_bus_master3/28-0000042b410e/w1_slave
29
1wirevz 8467   pi 1022r   REG       0,14     4096  3992 /sys/devices/w1_bus_master1/28-0000042b89e6/w1_slave

Die ganzen Sennsor_reads sind offen ... 1057!

von Welle 🧐 S. (w3llschmidt)


Lesenswert?

Ist das 'fclose ( fp )' ist eine Ebene zu tief ... ?!

1
double ds1820read(char *sensorid) {
2
3
  sprintf(fn, "/sys/bus/w1/devices/%s/w1_slave", sensorid );
4
5
  FILE *fp;  
6
  if  ( (fp = fopen ( fn, "r" )) == NULL ) {
7
  return(temp);
8
  }
9
  else 
10
  {
11
  
12
    fgets( crc_buffer, sizeof(crc_buffer), fp );
13
    if ( !strstr ( crc_buffer, crc_ok ) ) 
14
    {
15
      syslog(LOG_INFO, "%s", crc_buffer);
16
      syslog(LOG_INFO, "CRC check failed, SensorID: %s", sensorid);
17
    }
18
    else 
19
    {
20
    fgets( temp_buffer, sizeof(temp_buffer), fp );
21
    fgets( temp_buffer, sizeof(temp_buffer), fp );
22
      char *t;
23
      t = strndup ( temp_buffer +29, 5 ) ;
24
      temp = atof(t)/1000;
25
      return(temp);
26
    }
27
    
28
  fclose ( fp );  
29
  }
30
  
31
return(temp);
32
}

von Welle 🧐 S. (w3llschmidt)


Lesenswert?

Schon viel besser :-)

1
pi@rpi /tmp $ lsof -P -n -p 9603|more
2
COMMAND  PID USER   FD   TYPE DEVICE SIZE/OFF  NODE NAME
3
1wirevz 9603   pi  cwd    DIR  179,2     4096  8203 /tmp
4
1wirevz 9603   pi  rtd    DIR  179,2     4096     2 /
5
1wirevz 9603   pi  txt    REG  179,2    17579 21261 /usr/sbin/1wirevz
6
1wirevz 9603   pi  mem    REG  179,2   197040 16929 /usr/lib/arm-linux-gnueabihf/libgssapi_krb5.so.2.2
7
1wirevz 9603   pi  mem    REG  179,2    71528  1902 /lib/arm-linux-gnueabihf/libresolv-2.13.so
8
1wirevz 9603   pi  mem    REG  179,2     9812  1894 /lib/arm-linux-gnueabihf/libdl-2.13.so
9
1wirevz 9603   pi  mem    REG  179,2    92052 29589 /usr/lib/arm-linux-gnueabihf/libsasl2.so.2.0.25
10
1wirevz 9603   pi  mem    REG  179,2    46580 29621 /usr/lib/arm-linux-gnueabihf/liblber-2.4.so.2.8.3
11
1wirevz 9603   pi  mem    REG  179,2    89340 30334 /usr/lib/arm-linux-gnueabihf/librtmp.so.0
12
1wirevz 9603   pi  mem    REG  179,2    26368 16878 /usr/lib/arm-linux-gnueabihf/libkrb5support.so.0.1
13
1wirevz 9603   pi  mem    REG  179,2   126236  1886 /lib/arm-linux-gnueabihf/ld-2.13.so
14
1wirevz 9603   pi  mem    REG  179,2   198240 10493 /usr/lib/arm-linux-gnueabihf/libidn.so.11.6.8
15
1wirevz 9603   pi  mem    REG  179,2    26632  1893 /lib/arm-linux-gnueabihf/librt-2.13.so
16
1wirevz 9603   pi  mem    REG  179,2    38652  8685 /usr/lib/arm-linux-gnueabihf/libconfig.so.9.1.2
17
1wirevz 9603   pi  mem    REG  179,2   133096 30358 /usr/lib/arm-linux-gnueabihf/libssh2.so.1.0.1
18
1wirevz 9603   pi  mem    REG  179,2     9792  1189 /lib/arm-linux-gnueabihf/libcom_err.so.2.1
19
1wirevz 9603   pi  mem    REG  179,2     7332 32072 /usr/lib/arm-linux-gnueabihf/libcofi_rpi.so
20
1wirevz 9603   pi  mem    REG  179,2   363712 30403 /usr/lib/arm-linux-gnueabihf/libcurl.so.4.2.0
21
1wirevz 9603   pi  mem    REG  179,2  1196144  1892 /lib/arm-linux-gnueabihf/libc-2.13.so
22
1wirevz 9603   pi  mem    REG  179,2   264344 29622 /usr/lib/arm-linux-gnueabihf/libldap_r-2.4.so.2.8.3
23
1wirevz 9603   pi  mem    REG  179,2   289096 10809 /usr/lib/arm-linux-gnueabihf/libssl.so.1.0.0
24
1wirevz 9603   pi  mem    REG  179,2  1406216 10810 /usr/lib/arm-linux-gnueabihf/libcrypto.so.1.0.0
25
1wirevz 9603   pi  mem    REG  179,2    87792  5168 /lib/arm-linux-gnueabihf/libz.so.1.2.7
26
1wirevz 9603   pi  mem    REG  179,2   131372  1248 /lib/arm-linux-gnueabihf/libgcc_s.so.1
27
1wirevz 9603   pi  mem    REG  179,2   490896 10386 /lib/arm-linux-gnueabihf/libgcrypt.so.11.7.0
28
1wirevz 9603   pi  mem    REG  179,2   725164 10437 /usr/lib/arm-linux-gnueabihf/libgnutls.so.26.22.4
29
1wirevz 9603   pi  mem    REG  179,2   116462  1889 /lib/arm-linux-gnueabihf/libpthread-2.13.so
30
1wirevz 9603   pi  mem    REG  179,2   680756 16911 /usr/lib/arm-linux-gnueabihf/libkrb5.so.3.3
31
1wirevz 9603   pi  mem    REG  179,2   157584 16893 /usr/lib/arm-linux-gnueabihf/libk5crypto.so.3.1
32
1wirevz 9603   pi  mem    REG  179,2     9616 16860 /lib/arm-linux-gnueabihf/libkeyutils.so.1.4
33
1wirevz 9603   pi  mem    REG  179,2    11944 10448 /lib/arm-linux-gnueabihf/libgpg-error.so.0.8.0
34
1wirevz 9603   pi  mem    REG  179,2    54920 10928 /usr/lib/arm-linux-gnueabihf/libtasn1.so.3.1.16
35
1wirevz 9603   pi  mem    REG  179,2    59348 10639 /usr/lib/arm-linux-gnueabihf/libp11-kit.so.0.0.0
36
1wirevz 9603   pi    0uW  REG  179,2        5  1319 /tmp/1wirevz.pid
37
pi@rpi /tmp $

von Sven B. (scummos)


Lesenswert?

Tipp: Ordentliche Einrückung vermeidet solche Fehler! ;D

von Welle 🧐 S. (w3llschmidt)


Lesenswert?

Sven B. schrieb:
> Tipp: Ordentliche Einrückung vermeidet solche Fehler! ;D

:-D

von 900ss (900ss)


Lesenswert?

Henrik Wellschmidt schrieb:
> Schon viel besser :-)

Wenn du nur das fclose verschoben hast, dann ist da immer noch eine 
Lücke, wo die "Datei" offen bleibt.

Sven B. schrieb:
> Tipp: Ordentliche Einrückung vermeidet solche Fehler! ;D

Dem stimme ich zu. Mehrere "returns" machen den Code auch 
unübersichtlich. Einmal am Ende der Funktion recht auch. Dann muß man 
etwas anders codieren :)

von Welle 🧐 S. (w3llschmidt)


Lesenswert?

900ss D. schrieb:
> Henrik Wellschmidt schrieb:
>> Schon viel besser :-)
>
> Wenn du nur das fclose verschoben hast, dann ist da immer noch eine
> Lücke, wo die "Datei" offen bleibt.

Hi 900ss ... wo meinst ?

1
double ds1820read(char *sensorid) {
2
3
  sprintf(fn, "/sys/bus/w1/devices/%s/w1_slave", sensorid );
4
5
  FILE *fp;  
6
  if  ( (fp = fopen ( fn, "r" )) == NULL ) {
7
  fclose ( fp );
8
  return(temp);
9
  }
10
  else 
11
  {
12
  
13
    fgets( crc_buffer, sizeof(crc_buffer), fp );
14
    if ( !strstr ( crc_buffer, crc_ok ) ) 
15
    {
16
      syslog(LOG_INFO, "%s", crc_buffer);
17
      syslog(LOG_INFO, "CRC check failed, SensorID: %s", sensorid);
18
    }
19
    else 
20
    {
21
    fgets( temp_buffer, sizeof(temp_buffer), fp );
22
    fgets( temp_buffer, sizeof(temp_buffer), fp );
23
      char *t;
24
      t = strndup ( temp_buffer +29, 5 ) ;
25
      temp = atof(t)/1000;
26
      fclose ( fp );
27
      return(temp);
28
    }
29
30
  }
31
  
32
return(temp);
33
}

von 900ss (900ss)


Lesenswert?

Wenn der crc check fehl schlägt?

von Welle 🧐 S. (w3llschmidt)


Lesenswert?

900ss D. schrieb:
> Wenn der crc check fehl schlägt?

**sichmitderhandvornkoppklatscht**

Danke ;-)

Muss ich in jedem Fall 'return(temp)' zurückgeben ... ?

von thomil (Gast)


Lesenswert?

Hier noch ein paar anregungen zu ds1820read(), und zum ganzen Code im 
allgemeinen:

Installier dir das Programm "indent". Das rückt dir den code automatisch 
so ein, wie du es mit den command line parametern angibst. Dafür gibt es 
auch vordefinierte styles, wie den linux coding style. Dafür einfach 
mit:

indent -linux foo.c

und die Datei foo.c ist hinterher im Linux coding style eingerückt.

Wenn man das auf deine Funktion ds1820read() loslässt sieht diese schon 
mal viel übersichtlicher aus.

Ich war mal so frei und habe außerdem ein paar Kommentare in den Code 
eingefügt.
1
double ds1820read(char *sensorid)
2
{
3
4
  sprintf(fn, "/sys/bus/w1/devices/%s/w1_slave", sensorid);
5
6
  FILE *fp;
7
  if ((fp = fopen(fn, "r")) == NULL)
8
  {
9
    /* fclose hier unnoetig, da Datei nicht geoeffnet */
10
    fclose(fp);
11
    return (temp);
12
  }
13
  else
14
  {
15
    /* kann fehlschlagen, rueckgabe wert pruefen */
16
    fgets(crc_buffer, sizeof(crc_buffer), fp);
17
    if (!strstr(crc_buffer, crc_ok))
18
    {
19
      syslog(LOG_INFO, "%s", crc_buffer);
20
      syslog(LOG_INFO, "CRC check failed, SensorID: %s",
21
             sensorid);
22
      /* fp bleibt offen */
23
    }
24
    else
25
    {
26
      /* kann ebenfalls fehlschlagen */
27
      fgets(temp_buffer, sizeof(temp_buffer), fp);
28
      fgets(temp_buffer, sizeof(temp_buffer), fp);
29
      char *t;
30
      /* strndup() reserviert Speicher fuer string mit
31
       * malloc(). Dieser muss mit free() freigegeben werden */
32
      t = strndup(temp_buffer + 29, 5);
33
      temp = atof(t) / 1000;
34
      free(t); /* <-- fehlt in deinem code */
35
      fclose(fp);
36
      return (temp);
37
    }
38
  }
39
40
  return (temp);
41
}

Da du bei der Fehlerbehandlung außerdem etwas code duplizierst, würde 
ich den Code in etwa so schreiben:
1
/* Achtung: *val nur gueltig wenn Funktion 0 zurueckliefert */
2
int ds1820read(char *sensorid, double *val)
3
{
4
  FILE *fp;
5
  char *t;
6
  int ret = 0;
7
8
  sprintf(fn, "/sys/bus/w1/devices/%s/w1_slave", sensorid);
9
10
  if ((fp = fopen(fn, "r")) == NULL)
11
    return -errno;
12
13
  if (fgets(crc_buffer, sizeof(crc_buffer), fp) == NULL)
14
  {
15
    ret = -EOF;
16
    goto error;
17
  }
18
19
  if (!strstr(crc_buffer, crc_ok))
20
  {
21
    syslog(LOG_INFO, "%s", crc_buffer);
22
    syslog(LOG_INFO, "CRC check failed, SensorID: %s",
23
           sensorid);
24
25
    ret = -EIO;
26
    goto error;
27
  }
28
29
  if (fgets(temp_buffer, sizeof(temp_buffer), fp) == NULL)
30
  {
31
    ret = -EOF;
32
    goto error;
33
  }
34
35
36
  t = strndup(temp_buffer + 29, 5);
37
  *val = atof(t) / 1000;
38
  free(t);
39
40
error:
41
  fclose(fp);
42
  return ret;
43
}

Bitte melde dich an um einen Beitrag zu schreiben. Anmeldung ist kostenlos und dauert nur eine Minute.
Bestehender Account
Schon ein Account bei Google/GoogleMail? Keine Anmeldung erforderlich!
Mit Google-Account einloggen
Noch kein Account? Hier anmelden.