ATM bug found

Unlike 98% of the warnings of this type, this gcc warning does indeed seem to indicate a bug:

drivers/atm/zatm.c: In function ‘zatm_open’:
drivers/atm/zatm.c:919: warning: ‘pcr’ may be used uninitialized in this function

If alloc_shaper() argument 'unlimited' is true, then pcr is never assigned a value. However, the caller of alloc_shaper() always tests the pcr value, regardless of whether or not 'unlimited' is true.

Jeff

(trimmed cc:'s since, IMO, isn't really all that general interest)

Yeah, looks like a bug. Not very high-impact because:
1. it only results in garbled data in vcc->qos.txtp which doesn't really mean much for UBR vcc's anyway
2. zatm is a more-obscure-than-obscure piece of hardware. I'm sure 3c501.c has more users these days :-)

The fix is for alloc_shaper() should really do "*pcr = ATM_MAX_PCR" in the "if (ubr)" stanza. Chas, want to submit that in the next batch of patches?

-Mitch

i dont think you can do that. pcr gets assigned to .min_pcr and .min_pcr = ATM_MAX_PCR has a different meaning. it should probably be set to 0. there is no minimum or maximum pcr for ubr. you get what you get.

when unlimited is true, this means ubr. alloc_shaper() creates a queue to use for all ubr (best effort) traffic. ubr doesnt count against tx_bw so its handled a bit differently. alloc_shaper() should return a 0 for the pcr since this gets assigned to the vcc's qos parameters. min_pcr = 0 and max_pcr = 0 means "best effort".

still generates a warning from gcc though.

[ATM]: [zatm] always *pcr in alloc_shaper()

index c491ec4..083c5d3 100644
@@ -800,6 +800,7 @@ static int alloc_shaper(struct atm_dev *
 		i = m = 1;
 		zatm_dev->ubr_ref_cnt++;
 		zatm_dev->ubr = shaper;
+		*pcr = 0;
 	}
 	else {
 		if (min) {

The warning is bogus in this case, though -- the only way for "*pcr" to be unset is when alloc_shaper() returns non-zero